Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for updating Slurm options #277

Merged
merged 4 commits into from
Sep 6, 2024
Merged

Conversation

smoors
Copy link
Contributor

@smoors smoors commented Sep 1, 2024

this allows more flexibility in the Slurm options, depending on the easyconfig and/or architecture

below an example (using easybuild for parsing the easystack files, but you can of course roll your own)

example det_submit_opts.py
import os
import subprocess

from easybuild.framework.easystack import EasyStackParser


def get_orig_easystack(easystack, repo_path):
    """ write the original easystack file (before the diff was applied) """
    orig_easystack = f'{easystack}.orig'
    git_cmd = f'git -C {repo_path} show HEAD:{easystack}'.split()
    with open(os.path.join(repo_path, orig_easystack), 'w', encoding='utf-8') as outfile:
        subprocess.run(git_cmd, check=True, stdout=outfile)
    return orig_easystack


def det_submit_opts(job):
    """
    determine submit options from added easyconfigs
    Args:
        job (Job): namedtuple containing all information about job to be submitted

    Returns:
        (string): string containing extra submit options
    """
    easystack = 'vsc-2023a.yml'
    repo_path = job.working_dir
    orig_easystack = get_orig_easystack(easystack, repo_path)

    esp = EasyStackParser()
    orig_ecs = {x[0] for x in esp.parse(os.path.join(repo_path, orig_easystack)).ec_opt_tuples}
    pr_ecs = {x[0] for x in esp.parse(os.path.join(repo_path, easystack)).ec_opt_tuples}
    added_ecs = pr_ecs - orig_ecs
    print(f'added easyconfigs: {added_ecs}')

    submit_opts = [job.slurm_opts]
    for ec in added_ecs:
        if ec.startswith('PyTorch-'):
            submit_opts.append('--mem=32G')

    return ' '.join(submit_opts)

Copy link
Contributor

@trz42 trz42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for this contribution. It looks great.

I'd suggest to make this configurable. That is, one could add a setting, say allow_update_submit_opts in the [buildenv] section, which by default is False. Then the added code in tasks/build.py should only be run if the value of that setting is True.

The reason for this is that the code is run by the bot event handler. Hence, it would make it harder for us to protect the bot and it's data from unintended access. Making it configurable with a note that the setting should be used with care seems to be a reasonable trade-off.

@smoors
Copy link
Contributor Author

smoors commented Sep 4, 2024

@trz42 thanks a lot for the review.

config setting allow_update_submit_opts added in 4517ab2

Copy link
Contributor

@trz42 trz42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. Tested it and it just worked. Thanks @smoors !

@trz42 trz42 merged commit d17c46a into EESSI:develop Sep 6, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants