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

[feat] Add support for associating scheduler resources with an environment #3152

Merged
merged 19 commits into from
Apr 19, 2024

Conversation

ekouts
Copy link
Contributor

@ekouts ekouts commented Apr 4, 2024

This is a second attempt for this issue. I am missing the documentation + unittests but I would like first to agree on the api.

It would be very similar to the resources of the test. And the configuration would look like this.

site_configuration = {
    'systems': [
        {
            ...
            'partitions': [
                {
                    'resources': [
                        {
                            'name': 'switches',
                            'options': ['--switches={num_switches}']
                        },
                        {
                            'name': 'gres',
                            'options': ['--gres={gres}']
                        },
                        {
                            'name': 'uenv',
                            'options': [
                                '--uenv-file={file}',
                                '--uenv-mount={mount}',
                            ]
                        }
                    ]
                }
            ]
        }
    ],
    'environments': [
        {
            'name': 'myenv',
            'resources': {
                'uenv': {
                    'mount': 'one',
                    'file': 'two',
                }
            }
        }
    ]
}

That would translate to the following options in the job script:

#SBATCH --uenv-file=two
#SBATCH --uenv_mount=one

Closes #2991 .

@ekouts ekouts requested review from vkarak and teojgo April 4, 2024 09:35
@ekouts ekouts self-assigned this Apr 4, 2024
@pep8speaks
Copy link

pep8speaks commented Apr 4, 2024

Hello @ekouts, Thank you for updating!

Cheers! There are no PEP8 issues in this Pull Request!Do see the ReFrame Coding Style Guide

Comment last updated at 2024-04-18 22:44:40 UTC

Copy link

codecov bot commented Apr 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.67%. Comparing base (2d1eca4) to head (a278d9a).
Report is 1 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #3152   +/-   ##
========================================
  Coverage    86.66%   86.67%           
========================================
  Files           61       61           
  Lines        12134    12139    +5     
========================================
+ Hits         10516    10521    +5     
  Misses        1618     1618           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@vkarak vkarak modified the milestones: ReFrame 4.7, ReFrame 4.6 Apr 10, 2024
@ekouts ekouts marked this pull request as ready for review April 10, 2024 15:35
docs/config_reference.rst Outdated Show resolved Hide resolved
docs/config_reference.rst Outdated Show resolved Hide resolved
docs/config_reference.rst Outdated Show resolved Hide resolved
ekouts and others added 3 commits April 11, 2024 10:33
Co-authored-by: Theofilos Manitaras <manitaras@cscs.ch>
Co-authored-by: Theofilos Manitaras <manitaras@cscs.ch>
Co-authored-by: Theofilos Manitaras <manitaras@cscs.ch>
Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

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

I think we should omit completely the env_resources and simply enable environments to use any of the scheduler resources. This would simplify the implementation and make also more sense at the partition level, as conceptually it doesn't make sense to separate the two types of resources.

reframe/core/environments.py Outdated Show resolved Hide resolved
@ekouts ekouts requested a review from vkarak April 16, 2024 10:38
Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

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

Just some minor style changes.

docs/config_reference.rst Outdated Show resolved Hide resolved
docs/config_reference.rst Outdated Show resolved Hide resolved
unittests/resources/config/settings.py Outdated Show resolved Hide resolved
@vkarak vkarak changed the title Support injection of scheduler options from an environment Add support for associating scheduler resources with an environment Apr 18, 2024
@vkarak vkarak changed the title Add support for associating scheduler resources with an environment [feat] Add support for associating scheduler resources with an environment Apr 18, 2024
@ekouts
Copy link
Contributor Author

ekouts commented Apr 19, 2024

Thanks @vkarak for finishing the PR!

@ekouts ekouts merged commit db628b2 into reframe-hpc:develop Apr 19, 2024
25 checks passed
@ekouts ekouts deleted the feat/env_sched_bind_options_2 branch April 19, 2024 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Support injection of scheduler options from an environment
4 participants