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

JET update to enable fcst only run: #384

Closed
wants to merge 4 commits into from

Conversation

lgannoaa
Copy link
Contributor

Changes to be committed:
modified: env/JET.env
modified: modulefiles/module_base.jet
modified: sorc/build_gfs_util.sh
modified: sorc/build_gldas.sh
modified: sorc/build_workflow_utils.sh
modified: sorc/link_fv3gfs.sh

 Changes to be committed:
	modified:   env/JET.env
	modified:   modulefiles/module_base.jet
	modified:   sorc/build_gfs_util.sh
	modified:   sorc/build_gldas.sh
	modified:   sorc/build_workflow_utils.sh
	modified:   sorc/link_fv3gfs.sh
@lgannoaa lgannoaa self-assigned this Jul 28, 2021
Copy link
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.

See comments inline.

Comment on lines 15 to 26
# Load from official NCEPLIBS
module use -a /mnt/lfs4/HFIP/hfv3gfs/nwprod/NCEPLIBS/modulefiles
module load prod_util/v1.0.18
module load grib_util/v1.1.1
module load g2tmpl/1.6.0
module load crtm/2.3.0

# Add new path for netcdfp -- fix for netcdf HDF errors
setenv NCEPLIBS /lfs4/HFIP/hfv3gfs/gwv/l0701/lib/
module use -a /mnt/lfs4/HFIP/hfv3gfs/gwv/l0701/lib/modulefiles
module load esmflocal/8_0_1
module load netcdfp/4.7.4
Copy link
Contributor

Choose a reason for hiding this comment

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

Jet is using hpc-stack for the model. Please use those.
See jet related files here:
https://github.com/ufs-community/ufs-weather-model/tree/develop/modulefiles

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modification made as requested

@@ -8,7 +8,11 @@ USE_PREINST_LIBS=${USE_PREINST_LIBS:-"true"}
if [ $USE_PREINST_LIBS = true ]; then
export MOD_PATH=/scratch3/NCEPDEV/nwprod/lib/modulefiles
else
export MOD_PATH=${cwd}/lib/modulefiles
if [ $target = jet ]; then
export MOD_PATH=/lfs4/HFIP/hfv3gfs/nwprod/hpc-stack/libs/modulefiles/stack
Copy link
Contributor

Choose a reason for hiding this comment

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

using hpc-stack here, but not in module_base.jet. Is there a reason for this inconsistency?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why this if-block is needed or used.

Copy link
Contributor

Choose a reason for hiding this comment

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

@KateFriedman-NOAA Can you check why gldas needs this special treatment and others e.g. gsi, don't?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modification mad as requested

@@ -1,6 +1,8 @@
#!/bin/bash
set -eux

source ./machine-setup.sh > /dev/null 2>&1
Copy link
Contributor

Choose a reason for hiding this comment

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

This line is sourced on 14. I presume you are looking to set the variable target here. You don't need since the calling script build_all.sh sets this variable and passes it down. See: https://github.com/lgannoaa/global-workflow/blob/b19ccea00d59a4c8ea13d9f8ffb00fbe7bfc9d49/sorc/build_all.sh#L154

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modification made as requested

Changes to be committed:
	modified:   modulefiles/module_base.jet
	modified:   sorc/build_gldas.sh
	modified:   sorc/build_workflow_utils.sh
@lgannoaa lgannoaa requested a review from aerorahul July 28, 2021 15:16
Copy link
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.

More comments.

Comment on lines 5 to 8
export FCOMP=mpiifort

export CF=$FCOMP
export FC=$FCOMP
Copy link
Contributor

Choose a reason for hiding this comment

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

how can you set these before loading the compiler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The JET build with this version of OznMonBuild.jet was working.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is working, but not because of this line.

Setting these variables here is doing nothing since the compiler module is overwriting it.

Copy link
Member

Choose a reason for hiding this comment

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

We don't need the modulefiles/OznMonBuild.* and modulefiles/RadMonBuild.* modulefiles anymore, please delete all of them. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed as requested.

export ver=2.0.2
export FCOMP=mpiifort

export CF=$FCOMP
Copy link
Contributor

Choose a reason for hiding this comment

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

What is CF?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please check with the original developer. I use the same approach:
grep "export CF" *
OznMonBuild.jet:export CF=$FCOMP
OznMonBuild.orion:export CF=$FCOMP
OznMonBuild.wcoss_dell_p3:export CF=$FCOMP
RadMonBuild.jet:export CF=$FCOMP
RadMonBuild.orion:export CF=$FCOMP
RadMonBuild.wcoss_dell_p3:export CF=$FCOMP

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I don't think that is correct.
CF is not used in the GSI. I checked.
These flags being set here are also ignored.

I am sure you can eliminate lines 5-12 from all these files and it will be identical.

Also, if the PR is adding functionality provided by some other developers, please invite them in the review.

Copy link
Member

Choose a reason for hiding this comment

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

Same comment as above, please remove the modulefiles/OznMonBuild.* and modulefiles/RadMonBuild.* modulefiles. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed as requested.

Comment on lines 5 to 8
export FCOMP=mpiifort

export CF=$FCOMP
export FC=$FCOMP
Copy link
Contributor

Choose a reason for hiding this comment

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

same question as above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As requested, I put the answer above.

Copy link
Member

Choose a reason for hiding this comment

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

Same comment as above, please remove the modulefiles/OznMonBuild.* and modulefiles/RadMonBuild.* modulefiles. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed as requested.


if [ "$target" = "wcoss_dell_p3" ] || [ "$target" = "wcoss_cray" ] || [ "$target" = "jet" ] || [ "$target" = "hera" ] ; then
echo " "
echo " You are on WCOSS: $target "
Copy link
Contributor

Choose a reason for hiding this comment

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

You are not on WCOSS if you are on Hera or Jet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified to display Hera and Jet

@@ -0,0 +1,57 @@
#!/bin/sh
Copy link
Contributor

Choose a reason for hiding this comment

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

What part of the workflow runs this on Jet?
Is this necessary for research purposes?
This seems like an operational product.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gfs_util is not been used in JET fcst only run. It is just a place holder here to complete the build without error. If we are sure JET will never need the gfs_util, I can remove it. However, the build script will need to be modified to filter out JET target.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please delete this file.
We will revisit this when there is a need.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified build_gfs_util.sh to filter out JET for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

This script is called compile_gfs_util_jet.sh and yet it has targets for wcoss, wcoss_cray, hera etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

compile_gfs_util_jet.sh is deleted

Comment on lines +31 to +32
module load hdf5/1.10.6
module load netcdf/4.7.4
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to load these after png.
netcdf is used in w3 library.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am testing this item on Jet now.

Comment on lines +16 to +24
module load nco/4.9.1
module load cdo/1.9.5
module load gempak/7.4.2

module load prod_util/v1.0.18
module load grib_util/v1.1.1
module load g2tmpl/1.6.0
module load crtm/2.3.0
module load esmflocal/8_0_1
module load netcdfp/4.7.4
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make sure these are built with hpc-stack.
I know for a fact hpc-stack does not build netcdfp or esmflocal modules for these

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am testing this item on Jet now.

Copy link
Contributor

Choose a reason for hiding this comment

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

some of these are still not from hpc-stack. So if you are testing with these modules, then they are likely wrong.

@KateFriedman-NOAA
Copy link
Member

KateFriedman-NOAA commented Jul 28, 2021

@lgannoaa The FV3 build fails when run out-of-the-box from your PR branch on Jet. Please see the following, thanks!

my clone of your branch: /lfs4/HFIP/hfv3gfs/Kate.Friedman/git/feature-jet-fcst-only
log of FV3 build (as done via build_all.sh): /lfs4/HFIP/hfv3gfs/Kate.Friedman/git/feature-jet-fcst-only/sorc/logs/build_fv3.log

Also, the GLDAS and WW3 pre/post builds fail in an out-of-the-box build attempt:

  • The GLDAS is understandable since it has not been ported to Jet yet. The full cycled/DA port to Jet is being taken care of by Dave Huber in issue Port global-workflow to Jet #357.
  • The WW3 pre/post build failed because a jet modulefile for it needs to be added (please remedy, thanks!):
 -bash-4.2$ tail /lfs4/HFIP/hfv3gfs/Kate.Friedman/git/feature-jet-fcst-only/sorc/logs/build_ww3_prepost.log
+ '[' '!' -d ../exec ']'
++ pwd -P
+ finalexecdir=/mnt/lfs4/HFIP/hfv3gfs/Kate.Friedman/git/feature-jet-fcst-only/sorc/../exec
+ set +x
./build_ww3prepost.sh: line 13: ../modulefiles/modulefile.ww3.jet: No such file or directory

@@ -0,0 +1,57 @@
#!/bin/sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Please delete this file.
We will revisit this when there is a need.

@@ -0,0 +1,57 @@
#!/bin/sh
Copy link
Contributor

Choose a reason for hiding this comment

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

This script is called compile_gfs_util_jet.sh and yet it has targets for wcoss, wcoss_cray, hera etc.

@@ -18,4 +18,4 @@ echo ""
echo " Building ... Executables for GFS_UTILITIES "
echo ""

source ./compile_gfs_util_wcoss.sh
source ./compile_gfs_util_${target}.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

please revert.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed compile_gfs_util_jet.sh and reverted build_gfs_util.sh

@lgannoaa
Copy link
Contributor Author

Hi @junwang-noaa,
The fv3gfs build failed on Jet because there is no ufs_jet in fv3gfs.fd/modulefiles. The script compile.sh is looking for ufs_jet but ufs_jet.intel and ufs_jet.intel_debug are available. Please advice solution.

@aerorahul
Copy link
Contributor

Hi @junwang-noaa,
The fv3gfs build failed on Jet because there is no ufs_jet in fv3gfs.fd/modulefiles. The script compile.sh is looking for ufs_jet but ufs_jet.intel and ufs_jet.intel_debug are available. Please advice solution.

@lgannoaa
You need to add in build_fv3.sh

if [ $target = jet ]; then target=jet.intel ; fi

The modulefile for jet is present in the hash that you are checking out.
Please take a minute to look and investigate.

~/s/fv3gfs.fd ❯❯❯ git branch
* (HEAD detached at 9350745)
/s/fv3gfs.fd ❯❯❯ ls modulefiles/ufs_jet.intel 
modulefiles/ufs_jet.intel

@aerorahul
Copy link
Contributor

Hi @junwang-noaa,
The fv3gfs build failed on Jet because there is no ufs_jet in fv3gfs.fd/modulefiles. The script compile.sh is looking for ufs_jet but ufs_jet.intel and ufs_jet.intel_debug are available. Please advice solution.

@lgannoaa
You need to add in build_fv3.sh

if [ $target = jet ]; then target=jet.intel ; fi

The modulefile for jet is present in the hash that you are checking out.
Please take a minute to look and investigate.

~/s/fv3gfs.fd ❯❯❯ git branch
* (HEAD detached at 9350745)
/s/fv3gfs.fd ❯❯❯ ls modulefiles/ufs_jet.intel 
modulefiles/ufs_jet.intel

@lgannoaa
And if it doesn't build, then how was this tested prior to opening a PR?

@lgannoaa
Copy link
Contributor Author

Hi @DavidHuber-NOAA I created the following 6 module files to get gldas build working on Jet.
Please review them in JET:/mnt/lfs4/HFIP/hfv3gfs/Lin.Gan/PR/global-workflow/sorc/gldas.fd/modulefiles
gldas_rst.jet gldas_post.jet gldas_model.jet gldas2gdas.jet gdas2gldas.jet gldas_forcing.jet
If you agree with this solution, please let us know and update gldas package as needed.
Thanks

@lgannoaa
Copy link
Contributor Author

Hi @junwang-noaa,
The fv3gfs build failed on Jet because there is no ufs_jet in fv3gfs.fd/modulefiles. The script compile.sh is looking for ufs_jet but ufs_jet.intel and ufs_jet.intel_debug are available. Please advice solution.

@lgannoaa
You need to add in build_fv3.sh

if [ $target = jet ]; then target=jet.intel ; fi

The modulefile for jet is present in the hash that you are checking out.
Please take a minute to look and investigate.

~/s/fv3gfs.fd ❯❯❯ git branch
* (HEAD detached at 9350745)
/s/fv3gfs.fd ❯❯❯ ls modulefiles/ufs_jet.intel 
modulefiles/ufs_jet.intel

@lgannoaa
And if it doesn't build, then how was this tested prior to opening a PR?

I used a local modified slink as a workaround on JET during testing. I will modify build_fv3.sh using your solution.

@EdwardSafford-NOAA
Copy link
Contributor

I can confirm that the modulefiles/RadMonBuild.jet and modulefiles/OznMonBuild.jet are no longer needed. That change occurred when when we moved to building with cmake.

I'm afraid I can't offer much more; I don't have access to the jet machine.

@lgannoaa
Copy link
Contributor Author

I can confirm that the modulefiles/RadMonBuild.jet and modulefiles/OznMonBuild.jet are no longer needed. That change occurred when when we moved to building with cmake.

I'm afraid I can't offer much more; I don't have access to the jet machine.

Thank you @EdwardSafford-NOAA for clarification. There are removed.

@aerorahul
Copy link
Contributor

Hi @junwang-noaa,

The fv3gfs build failed on Jet because there is no ufs_jet in fv3gfs.fd/modulefiles. The script compile.sh is looking for ufs_jet but ufs_jet.intel and ufs_jet.intel_debug are available. Please advice solution.

@lgannoaa

You need to add in build_fv3.sh

if [ $target = jet ]; then target=jet.intel ; fi

The modulefile for jet is present in the hash that you are checking out.

Please take a minute to look and investigate.

~/s/fv3gfs.fd ❯❯❯ git branch

  • (HEAD detached at 9350745)

/s/fv3gfs.fd ❯❯❯ ls modulefiles/ufs_jet.intel

modulefiles/ufs_jet.intel

@lgannoaa

And if it doesn't build, then how was this tested prior to opening a PR?

I used a local modified slink as a workaround on JET during testing. I will modify build_fv3.sh using your solution.

So it was not tested entirely with this branch. When someone opens a PR, it is expected that they tested it with the branch.

@aerorahul
Copy link
Contributor

I can confirm that the modulefiles/RadMonBuild.jet and modulefiles/OznMonBuild.jet are no longer needed. That change occurred when when we moved to building with cmake.

I'm afraid I can't offer much more; I don't have access to the jet machine.

Thank you @EdwardSafford-NOAA. Your confirmation was all that was required.

@DavidHuber-NOAA
Copy link
Contributor

Hi @DavidHuber-NOAA I created the following 6 module files to get gldas build working on Jet.
Please review them in JET:/mnt/lfs4/HFIP/hfv3gfs/Lin.Gan/PR/global-workflow/sorc/gldas.fd/modulefiles
gldas_rst.jet gldas_post.jet gldas_model.jet gldas2gdas.jet gdas2gldas.jet gldas_forcing.jet
If you agree with this solution, please let us know and update gldas package as needed.
Thanks

@lgannoaa Could you add read permissions for me to /mnt/lfs4/HFIP/hfv3gfs/Lin.Gan/PR?

@lgannoaa
Copy link
Contributor Author

Hi @DavidHuber-NOAA I created the following 6 module files to get gldas build working on Jet.
Please review them in JET:/mnt/lfs4/HFIP/hfv3gfs/Lin.Gan/PR/global-workflow/sorc/gldas.fd/modulefiles
gldas_rst.jet gldas_post.jet gldas_model.jet gldas2gdas.jet gdas2gldas.jet gldas_forcing.jet
If you agree with this solution, please let us know and update gldas package as needed.
Thanks

@lgannoaa Could you add read permissions for me to /mnt/lfs4/HFIP/hfv3gfs/Lin.Gan/PR?

Done.

@KateFriedman-NOAA
Copy link
Member

@lgannoaa @DavidHuber-NOAA GLDAS is unrelated to this task/PR (GLDAS is not needed for free-forecast mode), please move GLDAS Jet port discussions to the related GLDAS repo issue for the port. Thanks!

@KateFriedman-NOAA
Copy link
Member

@lgannoaa Friendly reminder to follow the global-workflow commit message standards, see here:

https://github.com/NOAA-EMC/global-workflow/wiki/Development-of-Global-Workflow#commit-message-standards

Thanks!

@lgannoaa
Copy link
Contributor Author

I modified the build_gldas.sh, build_ww3prepost.sh, and build_gfs_util.sh to not build on Jet.

@KateFriedman-NOAA
Copy link
Member

I modified the build_gldas.sh, build_ww3prepost.sh, and build_gfs_util.sh to not build on Jet.

@lgannoaa Please do not disable those builds. The ww3prepost build just needs the modulefile added (see my comment above). The gfs_util and gldas builds will be taken care of separately. Thanks!

@lgannoaa
Copy link
Contributor Author

@lgannoaa Friendly reminder to follow the global-workflow commit message standards, see here:

https://github.com/NOAA-EMC/global-workflow/wiki/Development-of-Global-Workflow#commit-message-standards

Thanks!

@KateFriedman-NOAA you reported the GLDAS not build in the first place. Maybe you can also include a request for relocate that discussion in the same message instead of revisit the topic later.

@lgannoaa
Copy link
Contributor Author

I modified the build_gldas.sh, build_ww3prepost.sh, and build_gfs_util.sh to not build on Jet.

@lgannoaa Please do not disable those builds. The ww3prepost build just needs the modulefile added (see my comment above). The gfs_util and gldas builds will be taken care of separately. Thanks!

@KateFriedman-NOAA if individual build script is not disabled, the overall build process will have fail status. That may introduce confusion. I recommend to disable it (it is currently failed) for now and enable it when the build is actually working.

Disable JET build for unsupported package gfs_util, gldas, and ww3
Update python scripts in ush/rocoto
	deleted:    modulefiles/OznMonBuild.jet
	deleted:    modulefiles/RadMonBuild.jet
	modified:   sorc/build_fv3.sh
	modified:   sorc/build_gfs_util.sh
	modified:   sorc/build_gldas.sh
	modified:   sorc/build_ww3prepost.sh
	modified:   ush/rocoto/rocoto.py
	modified:   ush/rocoto/setup_expt_fcstonly.py
	modified:   ush/rocoto/setup_workflow_fcstonly.py
	modified:   ush/rocoto/workflow_utils.py
	deleted:    util/sorc/compile_gfs_util_jet.sh
@lgannoaa
Copy link
Contributor Author

@aerorahul I tested this package on JET under:
/mnt/lfs4/HFIP/hfv3gfs/Lin.Gan/PR/global-workflow
It build successfully.
It created expdir and xml file.

@aerorahul
Copy link
Contributor

@lgannoaa
Your branch is now out of date with develop which is adding files that are in direct conflict.
There are too many unresolved comments in this PR that is making it impossible to track what is meaningful v/s unnecessary or irrelevant.

Copy link
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.

This PR is not even close for a review, let alone testing and merge to develop.
New files are added over every iteration of the review.
The PR description lists 6 files, and yet there are 13 files with changes.

Comment on lines +17 to +22
if [ $target = jet ]; then
echo " GFS_UTIL does not support JET "
echo " "
exit
fi

Copy link
Contributor

Choose a reason for hiding this comment

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

don't need this block. please remove.
We don't check for every machine here, so no need to do check for jet here.

Comment on lines +7 to +12
if [ $target = jet ]; then
echo " GLDAS does not support JET "
echo " "
exit
fi

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove.

Comment on lines +13 to +19

if [ $target = jet ]; then
echo " WW3 does not support JET "
echo " "
exit
fi

Copy link
Contributor

Choose a reason for hiding this comment

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

please remove

@@ -0,0 +1,31 @@
#%Module#####################################################
Copy link
Contributor

Choose a reason for hiding this comment

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

we are not building this, so please remove.

Comment on lines +16 to +24
module load nco/4.9.1
module load cdo/1.9.5
module load gempak/7.4.2

module load prod_util/v1.0.18
module load grib_util/v1.1.1
module load g2tmpl/1.6.0
module load crtm/2.3.0
module load esmflocal/8_0_1
module load netcdfp/4.7.4
Copy link
Contributor

Choose a reason for hiding this comment

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

some of these are still not from hpc-stack. So if you are testing with these modules, then they are likely wrong.

@@ -33,17 +31,17 @@ def create_metatask(task_dict, metatask_dict):

strings = []

strings.append(f'<metatask name="{metataskname}">\n')
strings.append('<metatask name="%s">\n' % metataskname)
Copy link
Contributor

Choose a reason for hiding this comment

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

This file is an older version. Please merge develop into your branch.

@lgannoaa
Copy link
Contributor Author

This PR is not even close for a review, let alone testing and merge to develop.
New files are added over every iteration of the review.
The PR description lists 6 files, and yet there are 13 files with changes.

@aerorahul We have communicated all day on this PR. I worked with all your requests and changed my approach. It is normal to have files changed from this kind of activity. I am surprised to see this comment above.

Here is the justification for file changes:

  • OznMonBuild.jet and RadMonBuild.jet are the result of a group discussion today. Ed told us this can be removed.
  • build_fv3.sh is the result of a group discussion today. The module name is incorrect in JET fv3 build. You proposed a solution and I used it.
  • build_gfs_util.sh, build_gldas.sh, build_ww3prepost.sh also discussed today where it does not build on JET. Therefore it should be disabled on JET to allow the build process to complete.
  • The current global-workflow version of setup_expt_fcstonly.py, setup_workflow_fcstonly.py,  rocoto.py, and workflow_utils.py does not work on JET. There are syntax errors. I had to revert this auto-merged version back to the version I used before.

I recommend having a clear and complete request. For example,

  1. The hpc-stack is used in module_base.jet because of your request today. It was built without an error. There is no communication today on why it is wrong or what should be changed.
  2. If a package does not need to be built on JET, it should be clearly stated within the request to either disable the build script within the package or modify the main build all script. We can not just let it build and fail on JET.

@arunchawla-NOAA I worked with physics group to test my JET solution and they have accepted and are using it. I understand people may have different opinions on how things can be done. However, I do not understand why we see the statement this PR is not even close for a review.

@aerorahul
Copy link
Contributor

Because we are in the middle of a discussion. There are many issues to be resolved before it is ready for merge to develop.

GitHub provides a means to facilitate discussion by creating a Draft Pull Request. When the discussion is concluded, there is a button to mark this PR for Review.

If there are issues with syntax in the rocoto Python scripts, this is the first we heard about it in the PR. Please open a new issue for it as if these are syntax issues, they need to be handled urgently and separately.

The build for fv3 were clearly not correct as you were using a version built offline, probably with different modules than those in this PR. We caught that in this discussion.

Jet uses hpc-stack and we test the model routinely on jet with it. The modules you were loading are something else and unsupported. We caught that in this discussion.

The changes to disabling components needs to be handled in a better way than scattering if blocks across numerous scripts. That is not an opinion, it is practical programming for the future.

I am happy to finish the discussion offline.

The hash that this PR was opened on did not build on Jet. That in itself is a red flag. You admitted that you used a model executable built elsewhere to test this branch. That circumvents the build script in this repository.

I am not sure what the physics group tested for you, but I am fairly certain they did not checkout this branch, inspect how the model is being built and cleanly build and run their tests. Because if they had, they would have failed at the model.

@lgannoaa
Copy link
Contributor Author

Close this PR as requested and will open a draft next week.
Will start communicating offline.

BTW, physics group used my local copy of HOMEgfs on JET. This approach was the part of the JET task requirement assigned by Arun.

@lgannoaa lgannoaa closed this Jul 29, 2021
@aerorahul aerorahul mentioned this pull request Aug 2, 2021
zhanglikate pushed a commit to zhanglikate/global-workflow that referenced this pull request Oct 6, 2022
…n 2.7 support, rename v16beta to v16 and RT updates (NOAA-EMC#384)

* Update .gitmodules and submodule pointers for fv3atm and NEMS
* Remove Python 2.7 support from top-level CMakeLists.txt
* Reduce forecast length of test fv3_ccpp_gfs_v16_RRTMGP_c192L127 from 24h to 12h
* Rename v16beta to v16 everywhere except the public release documentation
* Bugfixes and missing changes
* Remove 'export CCPP_LIB_DIR=ccpp/lib' from all regression tests
* Update regression test baseline date tag to 20210128; skip-ci
* Update ecflow-python environment on cheyenne and jet; skip-ci
zhanglikate pushed a commit to zhanglikate/global-workflow that referenced this pull request Oct 6, 2022
* Updates to stochastic_physics_wrapper (NOAA-EMC#280)

Fix to stochastic_physics_wrapper to allow for random patterns to update at a longer time-step than model

Co-authored-by: Dom Heinzeller <climbfuji@ymail.com>

* Update for Jet, bug fixes in running with frac_grid=T and GFDL MP, and in restarting with frac_grid=T  (NOAA-EMC#304)

Update the modulefile for jet.intel to enable UPP v10.0.0. The hpc-stack v1.0.0 pre-release is used for this. Small changes are made to tests.rt.sh for jet.intel and gaea.intel (consistency with other platforms).

The submodule pointer update for fv3atm addresses bugs in the ufs-weather-model with frac_grid=T and GFDL microphysics, and with restarting the model when frac_grid=T (from @shansun6 and @SMoorthi-emc).

* Feature/update mom6 and retain b4b results for 025x025 resolution (NOAA-EMC#290)

* point MOM6 to new branch which corresponding to GFDL 20201022 commit
* modify fms_files.cmake and mom6_files.cmake to reflect changes in MOM6 code as this version of MOM6 contains some file deletion, new files being added and renaming of files
* manually set MOM6 parameters in order to retain origonal results for 0.25x0.25 resolution
* update MOM6 to include Bugfix for mom6solo to be built
* modify compile.sh to allow mom6solo compiling
* modify MOM_input_template for all resolutions based on GFDL MOM6-example main branch update on 20201022
* change executable permissions for CMakeLists.txt
* chmod 644 to 6 files Dom pointed out
* chmod for CMakeLists.txt and tests/compile.sh
* change baseline directpory to 20201202 in rt.sh

* Update CICE, Move regression test input outside baseline directory (NOAA-EMC#270)

*Updates CICE to most recent develop branch of NOAA-EMC
* Sets n_aero (number of aerosols) in ice_in_template to 0.
* removes trailing whitespace from ice_in
* moves regression test input outside baseline directory (ufs-weather PR NOAA-EMC#312)

Co-authored-by: Dusan Jovic <48258889+DusanJovic-NOAA@users.noreply.github.com>
Co-authored-by: Dom Heinzeller <dom.heinzeller@icloud.com>

* Updates to build for JEDI linking/control, add wcoss2 (#295)

* Build on wcoss2 (acorn)
* Use -march=core-avx2 instead of -xCORE-AVX2 on wcoss2
* Updates to build for JEDI linking/control
* Removed unnecessary include files and INLINE POST setting
* Updated to address PR suggestions.
* Add rt_acorn.conf. Change /lfs/h2 to /lfs/h1.
* Update .gitmodules and submodule pointer for fv3atm for code review and testing
* regression test results
* Updated .gitmodules and removed extraneous file
* Fixed .gitmodules and updated pointer for FV3
* Updated pointer to NEMS repo
Co-authored-by: Dusan Jovic <dusan.jovic@noaa.gov>
Co-authored-by: Dom Heinzeller <climbfuji@ymail.com>

* Final-final GFS v16 updates / restart reproducibility bugfixes (NOAA-EMC#325)

* Update .gitmodules and submodule pointer for fv3atm for code review and testing
* Add GFS v16 beta restart test, update stochastics test
* Update regression test baseline date tag to 20201214; skip-ci
* tests/rt.conf: bugfix, add missing 'fv3' to new stochy tests; skip-ci
* Regression test logs for gaea.intel, hera.gnu, hera.intel, jet.intel, orion.intel; skip-ci
* Run GFS v16beta tests also on wcoss; regression test logs for wcoss; skip-ci
* Regression test logs for cheyenne.intel and cheyenne.gnu
* Revert change to .gitmodules and update submodule pointer for fv3atm

* Add optional bulk flux calculation in ufs-datm (NOAA-EMC#266)


* Update NEMS DATM and CMEPS to allow the optional bulk flux formulation; add two tests using the option
* Update top level CMakeList.txt to have compile flags for MOM6 and CICE6 identical for ufs-cpld and ufs-datm
* Add optional configuration variable to nems.configure to specify the directory where CMEPS will write restarts
* Adds cheyenne tasking variables to default_vars and sets WW3_COMP to cheyenne for platform cheyenne.intel 

*NOTE: Baselines develop-20201215 exist on all platforms, regression tests were run against exactly that baseline on all systems except cheyenne.intel. On cheyenne.intel the tests were run against 20201214, and this baseline is identical to 20201215 (as per "diff -r develop-20201214 develop-20201215").

Co-authors:
@DusanJovic-NOAA
@aerorahul
@JessicaMeixner-NOAA

skip-ci

* Add 2 new tests for DATM-MOM6-CICE6 application (NOAA-EMC#332)

* Add the following 2 tests: datm_restart_cfsr, datm_debug_cfsr
* Add wcoss_dell_p3.log.
* Add Hera log, Orion log, wcoss_dell_p3 log.

* RRTMGP and Thompson MP coupling (NOAA-EMC#323)

* Feature branch with RRTMGP and Thompson MP
* Updated FV3/ccpp-physics. Added namelist and configuration for RRTMGP RTs using GSD physics.
* Updated FV3
* Update physics in FV3
* Updated baselines in rt.sh
* Updated RT logs. Updated FV3 physics submodule pointer.
* Updated FV3 hash and .gitmodules

* Regression test log for PR NOAA-EMC#323 for jet.intel (NOAA-EMC#336)

* Update modules with hpc-stack v1.1.0 (NOAA-EMC#319)

* Update modules with hpc-stack v1.1.0
* Minor bug fixes to CCPP UGWP

Co-authored-by: Dom Heinzeller <climbfuji@ymail.com>

* Replace old regional SDF with FV3_GFS_v15_thompson_mynn (NOAA-EMC#333)

* Replace old FV3_GFS_2017_gfdlmp_regional SDF for regional tests with FV3_GFS_v15_thompson_mynn.
* Final path to IC's and new results.  Also, input.nml updated.
* Update RegressionTests_wcoss_dell_p3.log
* Update RegressionTests_wcoss_cray.log
* Update RegressionTests_hera.intel.log
* Update RegressionTests_jet.intel.log
* Update RegressionTests_orion.intel.log
* Update RegressionTests_cheyenne* logs.
* Update RegressionTests_hera.gnu.log

* Feature/ww3update (NOAA-EMC#334)

This updates the WW3 submodule pointer to point to the top of the WW3 develop branch.
The path to WW3 inputs is changed to input-data-20201201/WW3_input_data_20201207/

* Remove IPD (step 1) (NOAA-EMC#331)

Make CCPP=Y the default in tests/compile.sh. Remove CCPP=Y from tests/rt*.conf and adjust formatting.
Update submodule pointer for MOM6 to include PR NOAA-EMC#341 ("Update MOM6 to GFDL's 20201218 commit")
Add modulefiles/wcoss_cray/fv3_debug (identical to modulefiles/wcoss_cray/fv3)
Fix broken utest (see NOAA-EMC#348)

* Update the format of rt.conf (NOAA-EMC#349)

Update the format of MACHINES column in rt.conf (and other .conf files). This column can be either empty, which means a test will run on all supported machines, or start with - or + sign to exclude or include specified machines explicitly.

* Add checkpoint restarts for ufs-cpld (NOAA-EMC#342)


* Adds 3 checkpoint restart tests for the ufs-cpld model
* Drops the existing c92mx025 restart test
* Adds cheyenne.intel as tested configuration for ufs-cpld and ufs-datm
* Fixes instances of srf_data* in various fv3_conf files

* add frac grid input, update and add additional cpld tests (NOAA-EMC#354)


* Updates FV3_input_frac to add both benchmark dates and L127 files
* Adds additional tests and restart tests for coupled model
* Sets all cpld tests to use frac grid input by default
* Removes all instances of  USE_LA_LI2016=True except for benchmark+wave configurations

* Remove unnecessary SIMD instruction sets for Jet, first round of cleanup in rt.conf, initialize cld_amt to zero for regional runs (dycore) (NOAA-EMC#353)

* Reduce SIMDMULTIARCH sets from four to two in cmake/Intel.cmake
* First cleanup of regression test config tests/rt.conf
* tests/rt.sh: reduce number of build jobs on jet.intel from 10 to 5
* Remove flags -f and -s from rt.sh, remove SET logic, remove corresponding column in all rt*conf files
* Remove tests/rt_acorn.conf and run GFS v15p2 and GFS v16beta DEBUG tests on all platforms

* Implementation of CCPP timestep_init and timestep_final phases (NOAA-EMC#337)

* Update .gitmodules and submodule pointer for fv3atm for code review and testing
* Update submodule pointer for fv3atm; skip-ci
* Don't try to compile all suites in DEBUG mode on cheyenne.intel, weird bug on compute nodes; skip-ci
* Don't try to compile all suites in DEBUG mode on wcoss_cray; skip-ci
* Regression test logs for cheyenne.gnu, cheyenne.intel, gaea.intel, hera.gnu, hera.intel, jet.intel, orion.intel; skip-ci
* Don't try to compile all suites in DEBUG mode on wcoss_dell_p3; skip-ci
* Regression test logs for wcoss_cray and wcoss_dell_p3
* Revert change to .gitmodules and update submodule pointer for fv3atm

* Update CMEPS  (NOAA-EMC#345)


* Update CMEPS for recent changes, including addition of new run "post" run phases to eliminate redundant mapping, multiple ice sheet capability and ocn->land ice dynamic mapping
* Add a new test fv3_gfs_v16_RRTMGP_c192L127

Co-authored-by: Jun Wang <junwang-noaa@users.noreply.github.com>

* Remove IPD steps 3 and 5 (NOAA-EMC#357)

Reduce SIMDMULTIARCH sets from four to two in cmake/Intel.cmake
* First cleanup of regression test config tests/rt.conf
* tests/rt.sh: reduce number of build jobs on jet.intel from 10 to 5; skip-ci
* Remove flags -f and -s from rt.sh, remove SET logic, remove corresponding column in all rt*conf files
* Update usage in rt.sh, add modulefiles/jet.intel/fv3_debug; skip-ci
* CCPP is default in cmake build
* Add debug modulefiles for linux.gnu and macosx.gnu
* Update submodule pointer for fv3atm
* Change logic in CMakeLists.txt and tests/compile.sh so that 32BIT=ON automatically sets DYN32=ON; skip-ci
* Move logic to set DYN32 - depending on 32BIT setting - to fv3atm
* Remove -DCCPP=ON from tests/compile.sh; update submodule pointer for fv3atm; skip-ci

* point fv3 to EMC develop branch (NOAA-EMC#377)

* update cpl gfsv16 tests, rrtmgp fix and bug fixes in cmeps (NOAA-EMC#378)

* update CMEPS, fix character length error for gnu compile
* add Dusan's fix for rt_utils.sh
* update cpl gfsv16 tests, replace seaice_newland.grb with global_slmask.t1534.3072.1536.grb, recover input.mom6.nml.IN, update input directory, update global thread and decomp tests, update fdiag for global control
* point to Dustins rrtmgp fix branch
* update input directory

Co-authored-by: denise.worthen <Denise.Worthen@noaa.gov>
Co-authored-by: Jun Wang <junwang-noaa@users.noreply.github.com>

* Update develop from NOAA-GSL: RUC ice, MYNN sfclay, stochastic land perturbations (NOAA-EMC#386)

* Update .gitmodules and submodule pointer for fv3atm for gsl/develop branch
* RUC ice for gsl/develop (replaces NOAA-EMC#47) (NOAA-EMC#49)Implementation of RUC LSM ice model in CCPP
* Squash-merge climbfuji:rucice_gfsv16dzmin into gsl/develop
* Add kice=9 to tests/tests/fv3_ccpp_rap and tests/tests/fv3_ccpp_hrrr
* Change NEW_BASELINE directory for gsl/develop to avoid conflicts with development work on the authoritative branches
* Add KICE=9 to tests/tests/fv3_ccpp_gsd_unified_ugwp and tests/tests/fv3_ccpp_gsd_drag_suite_unified_ugwp
* Revert change to .gitmodules and update submodule pointer for fv3atm
* Update gsl/develop from develop 2020/12/08 (NOAA-EMC#50)
* Updates to stochastic_physics_wrapper (NOAA-EMC#280)
Fix to stochastic_physics_wrapper to allow for random patterns to update at a longer time-step than model
* Update for Jet, bug fixes in running with frac_grid=T and GFDL MP, and in restarting with frac_grid=T  (NOAA-EMC#304)
Update the modulefile for jet.intel to enable UPP v10.0.0. The hpc-stack v1.0.0 pre-release is used for this. Small changes are made to tests.rt.sh for jet.intel and gaea.intel (consistency with other platforms).
The submodule pointer update for fv3atm addresses bugs in the ufs-weather-model with frac_grid=T and GFDL microphysics, and with restarting the model when frac_grid=T (from @shansun6 and @SMoorthi-emc).
* Land stochastic perturbations (NOAA-EMC#57)

* dycore options to add zero-gradient BC to reconstruct interface u/v and change dz_min as input (NOAA-EMC#369)

* Update fv3atm
* update ccpp control test forecast length to 24h
* remove rename command
* Add CI related changes
* Update RT logs
* Update RT log files
* Add the gaea RT log file
* Update the point of fv3atm
* Update fv3atm
Co-authored-by: Jun Wang <junwang-noaa@users.noreply.github.com>
Co-authored-by: MinsukJi-NOAA <minsuk.ji@noaa.gov>
Co-authored-by: Jun Wang <37633869+junwang-noaa@users.noreply.github.com>

* MOM6 bugfixes, GFDL update, update CDMBGWD settings; fix for restart reproducibility (without waves) when USE_LA_LI2016=True, sign error on fprec passed to ocean, GFDL update, resolution dependent cdmbgwd settings (NOAA-EMC#379)


* implements two MOM6 bugfixes in the NUOPC MOM6 cap to allow restart reproducibility when USE_LA_LI2016=True and to change the sign of the latent heat flux associated with frozen precipitation (fprec) exported to MOM6

* updates MOM6 to include the GFDL 20210120 main branch which contains EMC's wave coupling code, alone with some minor code standardization and documentation

* updates the cdmbgwd namelist settings for FV3 standalone tests at C96 and implements resolution dependent values for ufs-cpld tests

Co-authored-by: Ali <ali.abdolali@noaa.gov>

* Remove legacy gnumake build from fv3atm and NEMS, remove legacy Python 2.7 support, rename v16beta to v16 and RT updates (NOAA-EMC#384)

* Update .gitmodules and submodule pointers for fv3atm and NEMS
* Remove Python 2.7 support from top-level CMakeLists.txt
* Reduce forecast length of test fv3_ccpp_gfs_v16_RRTMGP_c192L127 from 24h to 12h
* Rename v16beta to v16 everywhere except the public release documentation
* Bugfixes and missing changes
* Remove 'export CCPP_LIB_DIR=ccpp/lib' from all regression tests
* Update regression test baseline date tag to 20210128; skip-ci
* Update ecflow-python environment on cheyenne and jet; skip-ci

* Update CMEPS for HAFS integration; add datm and coupled-model tests on Gaea (NOAA-EMC#401)


* Add HAFS support in NOAA-EMC/CMEPS 
* Add coupled and datm tests for Gaea.intel

Co-authored-by: Jun Wang <junwang-noaa@users.noreply.github.com>
Co-authored-by: Bin Li <Bin.Li@gaea13.ncrc.gov>

* Move LSM vegetation lookup tables into CCPP, clean up RUC snow cover on ice initialization (remove IPD step 2)  (NOAA-EMC#407)

* Regression test logs for all tier=1 platforms

* updates FMS to 2020.04.01 (NOAA-EMC#392)

* updates FMS to 2020.04.01
* fixes fms_files.cmake
* removes extra horiz_interp
* Workaround for FMS 2020.04.01 for Cheyenne with GNU 9.1.0, incl. regression test log
Co-authored-by: Mikyung Lee <mlee@Orion-login-1.HPC.MsState.Edu>
Co-authored-by: Dom Heinzeller <climbfuji@ymail.com>

* add optional mesh in MOM6; add dz_min and min_seaice as configurable variables for coupled model (NOAA-EMC#399)

*Implements an optional setting in the cpld and datm nems.configure files to specify whether the MOM6 cap should use a mesh or a grid

*Adds configurable settings for min_seaice to gfs_physics_nml and dz_min to fv_core_nml.

* UGWP v0 v1 combined (NOAA-EMC#396)

- combines the changes in PRs NOAA-EMC#360 and NOAA-EMC#382
- adds three regression tests `fv3_ccpp_gfsv16_ugwpv1 `, `fv3_ccpp_gfsv16_ugwpv1_warmstart` and `fv3_ccpp_gfsv16_ugwpv1_debug`
- contains updates and bugfixes for `nc_compare.py` and the CI tests from @MinsukJi-NOAA 
- update Python3 environment on jet.intel, gaea.intel, cheyenne.{intel,gnu}
- turn off (again) test `fv3_ccpp_decomp` on jet.intel, this test didn't work in the past, but recently it "passed", because the error checking with `nc_compare.py` failed silently and we didn't notice it

Co-authored-by: valery.yudin <valery.yudin@noaa.gov>
Co-authored-by: Michael Toy <michael.toy@noaa.gov>
Co-authored-by: MinsukJi-NOAA <minsuk.ji@noaa.gov>

* Update regression tests from GFSv15+Thompson to GFSv16+Thompson, include "Add one regional regression test in DEBUG mode. (NOAA-EMC#419)" (NOAA-EMC#421)

* Add one regional regression test in DEBUG mode.
* Update .gitmodules and submodule pointer for fv3atm for code review and testing
* Update regression tests from GFSv15+Thompson to GFSv16+Thompson
* Combine several COMPILE lines in tests/rt.conf and tests/rt_gnu.conf
* Regression test log for cheyenne.{gnu,intel},gaea.intel, hera.gnu, jet.intel,hera.intel,orion.intel;wcoss_cray and wcoss_dell_p3;

Co-authored-by: Phil Pegion <38869668+pjpegion@users.noreply.github.com>
Co-authored-by: jiandewang <jiande.wang@noaa.gov>
Co-authored-by: Denise Worthen <denise.worthen@noaa.gov>
Co-authored-by: Dusan Jovic <48258889+DusanJovic-NOAA@users.noreply.github.com>
Co-authored-by: Mark Potts <33099090+mark-a-potts@users.noreply.github.com>
Co-authored-by: BinLi-NOAA <bin.li@noaa.gov>
Co-authored-by: dustinswales <dustin.swales@noaa.gov>
Co-authored-by: Kyle Gerheiser <3209794+kgerheiser@users.noreply.github.com>
Co-authored-by: RatkoVasic-NOAA <37597874+RatkoVasic-NOAA@users.noreply.github.com>
Co-authored-by: Ali.Abdolali <37336972+aliabdolali@users.noreply.github.com>
Co-authored-by: Jun Wang <junwang-noaa@users.noreply.github.com>
Co-authored-by: Jun Wang <37633869+junwang-noaa@users.noreply.github.com>
Co-authored-by: XiaqiongZhou-NOAA <48254930+XiaqiongZhou-NOAA@users.noreply.github.com>
Co-authored-by: Ali <ali.abdolali@noaa.gov>
Co-authored-by: Bin Li <Bin.Li@gaea13.ncrc.gov>
Co-authored-by: MiKyung Lee <58964324+mlee03@users.noreply.github.com>
Co-authored-by: valery.yudin <valery.yudin@noaa.gov>
Co-authored-by: Michael Toy <michael.toy@noaa.gov>
Co-authored-by: MinsukJi-NOAA <minsuk.ji@noaa.gov>
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.

5 participants