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

Clean up and update regression test script and related files #202

Closed
wants to merge 6 commits into from

Conversation

MinsukJi-NOAA
Copy link
Collaborator

Description of changes

Clean up and update rt.sh and related files

Specific notes

Issues Fixed (include github issue #): #201

Are changes expected to change answers?

  • bit for bit (no)
  • more substantial (no)

Specific changes:

  • changes in parm directory (no)
  • changes in module files (no)
  • new tests added or removed (no)

Testing performed:

  • machines: Hera, Orion, Dell P3
  • details: Regression tests

Hashes used for testing:

  • NEMS:
  • CMEPS:
  • FV3:
  • MOM6:
  • CICE:
  • WW3:
  • FMS:
  • stochastic_physics:

@MinsukJi-NOAA MinsukJi-NOAA marked this pull request as ready for review October 2, 2020 21:42

else

if [[ $i =~ ufs.s2s ]]; then
d=$( cmp ${RTPWD}/${CNTLMED_DIR}/$i ${RUNDIR}/$i | wc -l )
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we leave this file and add extra netcdf file comparison using nccmp, cprnc or compare_ncfile.py (need to check efficiency) when cmp comparison shows difference?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ufs.s2s.* is still being compared because it is in LIST_FILES of tests/tests/. It's just that CNTLMED_DIR no longer exists.

Let me look into these other comparison methods. Where can I find info on compare_ncfile.py?

@junwang-noaa
Copy link
Collaborator

junwang-noaa commented Oct 7, 2020 via email

@junwang-noaa
Copy link
Collaborator

junwang-noaa commented Oct 7, 2020 via email

@junwang-noaa
Copy link
Collaborator

junwang-noaa commented Oct 7, 2020 via email

@MinsukJi-NOAA
Copy link
Collaborator Author

@junwang-noaa @DeniseWorthen

On Orion, comparing a single file ufs.s2s.cpl.r.2013-04-01-43200.nc (2.4G) took nccmp about 7.6 seconds as opposed to 0.6 seconds by cmp.

I think we should just use nccmp because:

  1. nccmp is available as a module on Hera and Orion
  2. We are just comparing 1 or 2 ufs.s2s.* files (and even that as a fallback), so maybe efficiency does not matter too much?
  3. I have had some problem with cprnc on Orion due to shared libraries (libnetcdff.so.6, libpthread-2.17.so, libc-2.17.so) not being found
  4. compare_ncfile.py requires netCDF4 python module, which at this point I am not sure where to find on Hera and Orion

From 3 and 4 above, unless they become a module at some point in time, they will have to reside in a person's directory, which is not a good idea.

Let me know your thoughts.

@DeniseWorthen
Copy link
Collaborator

Just to be clear, you tested two files which cmp shows are different (giving 0.6secs) but the same two files using nccmp -d shows they are the same (and it takes 7.6s).

I tried the same two sized files using my install of cprnc on Hera and got about 9secs, so it seems nccmp -d is slightly faster and doesn't have the porting issue.

@junwang-noaa
Copy link
Collaborator

junwang-noaa commented Oct 7, 2020 via email

@DeniseWorthen DeniseWorthen added the No Baseline Change PR should not change answers label Oct 9, 2020
@DeniseWorthen
Copy link
Collaborator

We are creating new baselines in PR #200, but we are leaving the comparisons of the ufs.s2s.cpl<> files commented out until we have implemented the additional check if ufs.s2s.cpl<> shows a difference. Is that correct?

@MinsukJi-NOAA
Copy link
Collaborator Author

We are creating new baselines in PR #200, but we are leaving the comparisons of the ufs.s2s.cpl<> files commented out until we have implemented the additional check if ufs.s2s.cpl<> shows a difference. Is that correct?

That is also my understanding. Does it mean that this PR #202 will have to generate a new baseline, correct?

@DeniseWorthen
Copy link
Collaborator

Yes, I think that is right since the files don't get copied to the baseline unless they're in the list of files.

@MinsukJi-NOAA
Copy link
Collaborator Author

I think this PR is no longer needed since Rahul and Dusan's weather PR (ufs-community/ufs-weather-model#217) is also making rt.sh related changes, and already running S2S based on new baselines. I propose we open a new PR in the weather repo later to implement nccmp.

ShanSunNOAA pushed a commit to ShanSunNOAA/ufs-s2s-model that referenced this pull request Oct 28, 2020
* add link_mpi=dbg for debug test
* add dbg_mt in debug option when OPENMP is on
* update log files for hera, orion, wcoss cray and dell; skip-ci
* update hera.gnu log file
* Set DEBUG_LINKMPI=OFF for cheyenne.intel and gaea.intel; fix post library dependencies for cheyenne.{intel,gnu} gnumake config
* Regression test log for cheyenne.intel

Co-authored-by: Jun Wang <[email protected]>
Co-authored-by: Dom Heinzeller <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No Baseline Change PR should not change answers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants