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

Make req_memory_per_node hook print a warning if memory resource not configured in ReFrame config #182

Merged
merged 3 commits into from
Sep 18, 2024

Conversation

casparvl
Copy link
Collaborator

@casparvl casparvl commented Sep 17, 2024

Fixes #181

Haven't had time to test yet...

@casparvl casparvl marked this pull request as draft September 17, 2024 14:20
@casparvl
Copy link
Collaborator Author

casparvl commented Sep 17, 2024

Looks as intended when running on Azure MC cluster:

$ reframe -n ESPRESSO.*P3M -t "1_node" --run
...
[ RUN      ] EESSI_ESPRESSO_P3M_IONIC_CRYSTALS %module_name=ESPResSo/4.2.1-foss-2023a %device_type=cpu %scale=1_node /4a9e965c @Magic_Castle_Azure:aarch64-neoverse-N1-16c-62gb+default
WARNING: Your ReFrame configuration file does not specify any resource called 'memory' for this partition  (aarch64-neoverse-N1-16c-62gb). Without this, an explicit memory request cannot be made from the scheduler. This test will run, but it may result in an out of memory error. Please specify how to requst memory (per node) from your resource scheduler by defining a resource 'memory' in your ReFrame configuration file for this partition. For a SLURM system, one would e.g. define: 'resources': [{'name': 'memory', 'options': ['--mem={size}']}]

@casparvl casparvl marked this pull request as ready for review September 17, 2024 19:42
@casparvl
Copy link
Collaborator Author

On a system that does have the memory specified as resources:

$ reframe -n ESPRESSO.*P3M -t "1_node" --run
...
[----------] start processing checks
[ RUN      ] EESSI_ESPRESSO_P3M_IONIC_CRYSTALS %module_name=ESPResSo/4.2.2-foss-2023b %device_type=cpu %scale=1_node /3b90ffd6 @vega:cpu+default
[ RUN      ] EESSI_ESPRESSO_P3M_IONIC_CRYSTALS %module_name=ESPResSo/4.2.2-foss-2023a %device_type=cpu %scale=1_node /f1621689 @vega:cpu+default
[ RUN      ] EESSI_ESPRESSO_P3M_IONIC_CRYSTALS %module_name=ESPResSo/4.2.1-foss-2023a %device_type=cpu %scale=1_node /4a9e965c @vega:cpu+default

I.e. it doesn't print a warning - as expected.

Imho this PR is ready to be merged.

@casparvl
Copy link
Collaborator Author

casparvl commented Sep 17, 2024

Failing CI is fixed in #183, that should be merged first.

Edit: was just merged by @bedroge . Rerunning CI check now, I'm assuming it'll pass now.

Copy link
Collaborator

@satishskamath satishskamath left a comment

Choose a reason for hiding this comment

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

This warning is required. lgtm!

@satishskamath satishskamath merged commit 45808ba into EESSI:main Sep 18, 2024
10 checks passed
@casparvl casparvl changed the title Make hook print a warning Make req_memory_per_node hook print a warning if memory resource not configured in ReFrame config Sep 19, 2024
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.

Extra memory request silently fails if resource is not set in config
2 participants