Skip to content

Conversation

@rsarm
Copy link
Contributor

@rsarm rsarm commented Feb 23, 2021

Closes #1734

@rsarm rsarm added this to the ReFrame 3.5.0 milestone Feb 23, 2021
@rsarm rsarm requested review from teojgo and vkarak February 23, 2021 08:36
@rsarm rsarm self-assigned this Feb 23, 2021
Copy link
Contributor

@teojgo teojgo left a comment

Choose a reason for hiding this comment

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

We wil also need a unittest for this.

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 don't think this PR is correct in its principle. These commands are partition specific and not test-specific. I don't think we should anything to the test API. Instead, these commands must be part of the configuration of the partition. This is what I understand from the issue: every script submitted to that partition needs to have a module purge.

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.

The implementation looks good. I have a suggestion about the name of the option and we still need the following:

  1. A unit test or adapt test_prepare
  2. Documentation (both in the configuration reference and here (we need to update the script template).

@teojgo
Copy link
Contributor

teojgo commented Mar 3, 2021

@rsarm, @vkarak do you think that this is the case for both build/run scripts? I have the feeling that this is needed only for the compute nodes, at least on #1734

@vkarak
Copy link
Contributor

vkarak commented Mar 3, 2021

@teojgo I think you are correct here. We should use it also for the build scripts, since for the moment we say that we submit to the same partition, so users would expect to see that in the build scripts, too, since they submit in the same partition.

@teojgo
Copy link
Contributor

teojgo commented Mar 3, 2021

@teojgo I think you are correct here. We should use it also for the build scripts, since for the moment we say that we submit to the same partition, so users would expect to see that in the build scripts, too, since they submit in the same partition.

Yes for the moment it is used for both.

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.

lgtm

@vkarak vkarak requested a review from teojgo March 4, 2021 13:18
@vkarak vkarak changed the title [feat] Add option to include commands in the job script before the modules are loaded [feat] Add a new prepare_cmds partition configuration parameter which allows to emit commands before loading any environment Mar 4, 2021
@vkarak
Copy link
Contributor

vkarak commented Mar 4, 2021

@branfosj With this feature you will now be able to define your partition as follows:

{
    'name': 'mypart',
    'scheduler': 'slurm',
    'launcher': 'srun',
    'prepare_cmds': ['module purge'],
    'modules': ['bluebear']
}

The prepare_cmds will always be emitted before any commands that load the test environment. Let us know if this is sufficient.

@branfosj
Copy link

branfosj commented Mar 4, 2021

Thanks

@vkarak vkarak merged commit 13a7261 into reframe-hpc:master Mar 4, 2021
@rsarm rsarm deleted the feat/preload-cmds branch March 10, 2021 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Option to include commands in the job script before the modules are loaded

5 participants