-
Notifications
You must be signed in to change notification settings - Fork 133
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 conda environment, fix Makefile and Macros, implement -nomodules
#296
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
16 tasks
The CFLAG_HOST variable was added in CICE-Consortium#257, and some Makefile logic was added to define the variable to be blank if the included Macros file does not define it. However, the Make syntax is wrong, as 'ifndef' takes a variable name and not a reference to a variable [1], so ifndef $(CFLAGS_HOST) should have been written ifndef CFLAGS_HOST The effect of this error is to invert the logic of the check (!) since if CFLAGS_HOST is defined, Make will check if a variable with name equal to whatever CFLAGS_HOST is defined to be exists, which will most probably be false, and the conditional will then evaluate to true and CFLAG_HOSTS will be redefined to be blank, defeating the whole purpose of the flag. Since there's no harm in Make referencing and empty variable, fix this by just removing the conditional check. [1] https://www.gnu.org/software/make/manual/html_node/Conditional-Syntax.html
These two files used outdated environment variables for threading ("compile_threaded") and PIO ("IO_TYPE"). Replace these with the correct variables, ICE_THREADED and ICE_IOTYPE. Mirrors CICE-Consortium/CICE#396
Some env files wrap the calls for loading the build and run environment in a conditional based on the argument `-nomodules`, so that scripts that source the env file just to get the variables defined in it do not run slower because they have to load the environment. This logic was copied from CICE but was not implemented in the Icepack scripts icepack.setup and setup_run_dirs.csh. Add the argument '-nomodules' so that the logic becomes effective.
On Ubuntu and its derivatives, the csh shell at /bin/csh, which is used in the shebangs of the Icepack scripts, is not compatible with conda. Mention that in the documentation and explain how to install tcsh as an alternative, and configure the system such that /bin/csh points to /bin/tcsh.
apcraig
approved these changes
Feb 11, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested this on my Mac laptop using the updated documentation and everything works well. I will merge this in the next day or so unless there are any other comments.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR checklist
Add a conda environment for macOS and Linux, mirroring Add conda environment to run on personal computers CICE#393
P. Blain
@apcraig
Did not run the tests suite. I tested that the procedure works on Mac and Linux.
This PR implements the conda environment for Icepack. The
environment.yml
file is simpler since Icepack has less dependencies. A few points:CFLAGS_HOST
, as in Add conda environment to run on personal computers CICE#393icepack.setup
andsetup_run_dirs.csh
to implement the-nomodules
logic which was already used by some env files but was ineffective.icepack.launch.csh
to useomplace
on cheyenne, as was done in Macros fixes and diag_type fix CICE#396. @apcraig I'll let you push directly to this branch if you want to add that, although I think it can be left for later as all test suites have1x1
PEs so merging this PR will not switch on any threading on cheyenne.