Skip to content

Optionally add links to load_compass_env.sh in test cases #617

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

Merged
merged 2 commits into from
Jul 23, 2020

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Jul 1, 2020

This is specified either in the config file or at the command line.

This is specified either in the config file or at the command
line.
This will allow different cores to be synced with different
versions of the compass conda environment and will allow each
core to update the version its develop branch as needed.
@xylar
Copy link
Collaborator Author

xylar commented Jul 1, 2020

@matthewhoffman, this would add a capability we've already been using a little bit on ocean/develop to have a script that knows about machines we commonly use COMPASS on and which can be sourced to load the appropriate conda environment for COMPASS. An example of such a script is here, though I will need to move it into the ocean subdirectory in a separate PR.

If this functionality sounds useful to you, you will likely want to add a copy of the script to landice and update the version number for the compass conda env as needed. If it doesn't sound useful, that's fine, and the only review I would request from you is to make sure this change doesn't cause you any trouble (either by inspection or by running a landice test case).

@xylar
Copy link
Collaborator Author

xylar commented Jul 1, 2020

@mark-petersen, I am having trouble with other testing I'm doing because of conflicts between develop and ocean/develop. I think our quick-and-dirty approach of modifying setup_testcase.py in ocean/develop has turned out to be a bad move and I will strongly advocate against doing that in the future.

I realized in the process of putting in the changes we have made (which, after reversion of some of my MPI work, is just linking load_compass_env.sh) that there will be issues if different cores try to update the same load_compass_env.sh. Instead, we need one per core and it will be clear which core owns each file and which branch it should be modified in. In #618, I have moved load_compass_env.sh into the ocean directory on ocean/develop.

@xylar
Copy link
Collaborator Author

xylar commented Jul 1, 2020

Testing

I tested by setting up the ocean/global_coean/QU240wISC/init test case with and without the --link_load_compass flag (after merging this branch into #618). Without the flag, there is no load_compass_env.sh link in the test case, as expected. With the flag, the link is there and sourcing it loads the expected compass_0.1.6 environment on Grizzly.

@xylar xylar removed the in progress label Jul 1, 2020
@matthewhoffman
Copy link
Member

@xylar , this sounds useful. I will try it out.

@xylar
Copy link
Collaborator Author

xylar commented Jul 1, 2020

@matthewhoffman, keep in mind that the more recent compass_* environments include MPI, and that causes problems if you load things in the wrong order. The order of operations would be to first source load_compass_env.sh and then load the modules for the system compilers and MPI (so they take priority). I know this is a pain but I haven't figured out a better way.

Copy link
Contributor

@mark-petersen mark-petersen left a comment

Choose a reason for hiding this comment

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

@xylar this is fine with me if you've tested it. I've seen those merge conflicts as well, so it would be nice to merge it in.

@matthewhoffman any comments, or should we proceed?

Copy link
Member

@matthewhoffman matthewhoffman left a comment

Choose a reason for hiding this comment

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

@xylar , thanks for pointing this out to me. I don't see any problems with adding this. I'll wait and see how you use it in the ocean before thinking about adapting it for our testing.

@mark-petersen mark-petersen merged commit fd19a9e into MPAS-Dev:develop Jul 23, 2020
@xylar xylar deleted the compass/link_load_compass_env branch August 4, 2020 06:04
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.

3 participants