Skip to content

Refactor modulefiles#482

Closed
DusanJovic-NOAA wants to merge 8 commits into
ufs-community:developfrom
DusanJovic-NOAA:module_common
Closed

Refactor modulefiles#482
DusanJovic-NOAA wants to merge 8 commits into
ufs-community:developfrom
DusanJovic-NOAA:module_common

Conversation

@DusanJovic-NOAA
Copy link
Copy Markdown
Collaborator

PR Checklist

  • Ths PR is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR. Please consult the ufs-weather-model wiki if you are unsure how to do this.

  • This PR has been tested using a branch which is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR

  • An Issue describing the work contained in this PR has been created either in the subcomponent(s) or in the ufs-weather-model. The Issue should be created in the repository that is most relevant to the changes in contained in the PR. The Issue and the dependent sub-component PR
    are specified below.

  • If new or updated input data is required by this PR, it is clearly stated in the text of the PR.

Description

All currently supported module files have been updated to load file which specifies all common libraries.
I'm not sure what to do about odin and s4.intel.

Testing

How were these changes tested? What compilers / HPCs was it tested with? Are the changes covered by regression tests? (If not, why? Do new tests need to be added?) Have regression tests and unit tests (utests) been run? On which platforms and with which compilers? (Note that unit tests can only be run on tier-1 platforms)

  • hera.intel
  • hera.gnu
  • orion.intel
  • cheyenne.intel
  • cheyenne.gnu
  • gaea.intel
  • jet.intel
  • wcoss_cray
  • wcoss_dell_p3

Dependencies

No.

Copy link
Copy Markdown
Contributor

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

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

Most changes look good to me.
I have 2 comments:

  1. see comment under ufs_hera.gnu
  2. see comment regarding the use of MACHINE_ID.

Comment thread modulefiles/ufs_hera.gnu
Comment thread tests/compile.sh
set +x
if [[ $MACHINE_ID == macosx.* ]] || [[ $MACHINE_ID == linux.* ]]; then
source $PATHTR/modulefiles/${MACHINE_ID}/fv3
source $PATHTR/modulefiles/ufs_${MACHINE_ID}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MACHINE_ID also contains compiler, which is a little misleading.
e.g. MACHINE_ID = hera.gnu or hera.intel or orion.intel or wcoss_dell_p3.
There is a lack of consistency on what MACHINE_ID is supposed to mean.
Sometimes it is machine.compiler and sometimes it is just machine.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oh yes, that is "historical", we have moved everything except the wcoss platforms to contain compiler information. I don't know why this didn't happen for wcoss. Agreed that machine_id is not the best name for it, but as long as we are consistent ... we could go through all scripts (and regression test files) and rename it to "platform" maybe? In a separate PR ...

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the changes of making consistent MACHINE_ID might not be trivial, it's better to do it in separate PR.

@MinsukJi-NOAA
Copy link
Copy Markdown
Contributor

MinsukJi-NOAA commented Mar 25, 2021

I will add corresponding changes to the utest script.

Copy link
Copy Markdown
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

I would just rename the odin s4 modulefiles as you did for the others. It's the same as for stampede2, it will work for a while when someone updates it, and then age off.

DusanJovic-NOAA and others added 2 commits March 25, 2021 15:40
* Add utest change for hera
* Add utest change for orion
* Add utest change for dell p3
Copy link
Copy Markdown
Contributor

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

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

Looks good

Comment thread tests/compile.sh
set +x
if [[ $MACHINE_ID == macosx.* ]] || [[ $MACHINE_ID == linux.* ]]; then
source $PATHTR/modulefiles/${MACHINE_ID}/fv3
source $PATHTR/modulefiles/ufs_${MACHINE_ID}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the changes of making consistent MACHINE_ID might not be trivial, it's better to do it in separate PR.

Comment thread tests/utest

source $PATHTR/NEMS/src/conf/module-setup.sh.inc
module use $PATHTR/modulefiles/${MACHINE_ID}
module load fv3
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I am curious, do we not need to load the module file when running utest on hera?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similar to rt.sh, modulefiles are loaded by compile.sh and run_test.sh.

@DusanJovic-NOAA DusanJovic-NOAA deleted the module_common branch April 15, 2021 12:37
pjpegion pushed a commit to NOAA-PSL/ufs-weather-model that referenced this pull request Apr 4, 2023
epic-cicd-jenkins pushed a commit that referenced this pull request Apr 17, 2023
* Add level for data: section indicating ics_lbcs.

* Add the NaturalEarth shape files.

* Add plotting to the workflow.

* Fix some deprecation warnings.

* Don't plot 500mb vars if it is not available.

* Add modulefiles for plot_allvars task.

* Add a test case for plotting graphics.

* Add diff plotting.

* Remove ush/Python folder.

* Turn off plotting by default.

* Bug fix plot_diff call.

* Add missing diff plot script.

* Make plotting work in NCO mode.

* Avoid positional arguments in the plotting scripts.
Some bug fixes.

* Make COMOUT_REF a template to compare multiple dates and cycles correctly.

* Use logging to capture output from plot scripts in log files.

* Turn off debug level, too much information.

* Ignore warnings in plotting.

* Bug fix diff plotter: mismatch in number of ticks, and ticklabels.

* Increase wallclock time, bugfix orion FIXshp.

* Add some default values to config.community/nco for convenience.
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.

6 participants