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

enable feature/coupled-crow to work on stampede2 #261

Conversation

JianKuang-Intelsat
Copy link
Contributor

In addition to the stampede2 porting, this branch also switch to hpc-stack on Orion. Please try run this branch on both Orion and stampede2.

Copy link
Contributor

@JessicaMeixner-NOAA JessicaMeixner-NOAA left a comment

Choose a reason for hiding this comment

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

Is there an example directory for output with this change for orion? I'm guessing someone else will need to test for the UPP changes on hera as well. Do I need to do this or is someone else available to test this on hera?

I think the fix files need to be updated on Stampede.

@@ -0,0 +1,68 @@
#%Module######################################################################
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 not needed, is there an example test case on orion since the orion modules were updated?

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 same case file for the trunk works here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree this should be removed

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 needs to be removed before PR is merged.

sorc/link_fv3gfs.sh Outdated Show resolved Hide resolved
@@ -0,0 +1,218 @@
# Create a test function for sh vs. bash detection. The name is
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this file needed?

workflow/cases/coupled_free_forecast_wave_test.yaml Outdated Show resolved Hide resolved
@JianKuang-Intelsat
Copy link
Contributor Author

@JessicaMeixner-NOAA Thanks for these comments. I think since the primary purpose of this PR is to enable stampede2 running (stampede2 ONLY has hpc-stack modules, as of I know), and coupled-crow is functional on Hera as of now, we should leave hera-hpcstack adoption to a future PR.

@JessicaMeixner-NOAA
Copy link
Contributor

@JessicaMeixner-NOAA Thanks for these comments. I think since the primary purpose of this PR is to enable stampede2 running (stampede2 ONLY has hpc-stack modules, as of I know), and coupled-crow is functional on Hera as of now, we should leave hera-hpcstack adoption to a future PR.

Agreed, however I think hera still needs to be tested for this PR because the version of ncep_post and how it was built is changed. Therefore, I think we need to ensure nothing was broken on hera. In addition, there are updates for orion in this PR, so we should make sure it's been tested there as well, even though that was not the main goal of this PR, which as you stated is the enable stampede2.

@JianKuang-Intelsat
Copy link
Contributor Author

@JessicaMeixner-NOAA Yes. I agree.

remove excess files
@JessicaMeixner-NOAA
Copy link
Contributor

@JianKuang-NOAA I noticed this branch is out of date when I was checking it out to test on hera, can you please merge the latest feature/coupled-crow so I can test this on hera -- if someone else is already running the test on hera, please let me know.

@JianKuang-Intelsat
Copy link
Contributor Author

@JessicaMeixner-NOAA Thanks for your mention. I just merged it.

@JessicaMeixner-NOAA
Copy link
Contributor

@JianKuang-NOAA my attempt on hera failed, but the error seemed weird so I'm trying again with a fresh clone/build and will let you know how that goes. In the meantime my test on orion failed for ocean and the gridded wave post: /work/noaa/marine/jmeixner/p6stampede/test01/COMROOT/test01/logs/2013040100
It's my understanding that those currently work on orion, so hopefully those can be fixed before merging this PR in.

Copy link
Contributor

@JessicaMeixner-NOAA JessicaMeixner-NOAA left a comment

Choose a reason for hiding this comment

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

The re-test on hera went fine and @LydiaStefanova-NOAA checked the atm-post and the grib files were identical to what is in the current feature/coupled-crow so that's all fine. The orion issues (wave post and ocean post not running) need to be addressed before merging.

@WalterKolczynski-NOAA
Copy link
Contributor

WalterKolczynski-NOAA commented Mar 2, 2021

@WalterKolczynski-NOAA I am waiting for the update for ocean post enabling it to run using hpc-stack.

reg2grb2 or something else? Because the PR migrating reg2grb2 to hpc-stack was merged last week (#273).

@JianKuang-Intelsat
Copy link
Contributor Author

@WalterKolczynski-NOAA Yes. That's great. I will merge-in the updated coupled-crow and re-enable this PR later today. And invite you to review.

…e/hpcstack

Conflicts:
	modulefiles/module_base.orion
Copy link
Contributor

@WalterKolczynski-NOAA WalterKolczynski-NOAA left a comment

Choose a reason for hiding this comment

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

Minor changes except for getting hpc-stack updated.

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

Choose a reason for hiding this comment

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

Agree this should be removed

modulefiles/module_base.stampede Show resolved Hide resolved
modulefiles/modulefile.reg2grb2.stampede Outdated Show resolved Hide resolved
modulefiles/modulefile.ww3.stampede Outdated Show resolved Hide resolved
sorc/build_ncep_post.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@WalterKolczynski-NOAA WalterKolczynski-NOAA left a comment

Choose a reason for hiding this comment

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

Approved once the component updates are in.

@JianKuang-Intelsat
Copy link
Contributor Author

@WalterKolczynski-NOAA This PR is now up-to-date. Please take a test on Hera and Orion.

cd ${pwd}/ufs_utils.fd/sorc
./link_fixdirs.sh $RUN_ENVIR $machine
fi
#if [ -d ${pwd}/ufs_utils.fd ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is for debug purpose. I am restoring it now.

@@ -0,0 +1,68 @@
#%Module######################################################################
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 needs to be removed before PR is merged.

@JianKuang-Intelsat
Copy link
Contributor Author

@JessicaMeixner-NOAA @WalterKolczynski-NOAA I addressed Jessica's comments. Please take a test and approve this PR if ready. Thanks.

@WalterKolczynski-NOAA
Copy link
Contributor

I'm on leave until Monday, so I might not get to it until then.

@WalterKolczynski-NOAA
Copy link
Contributor

@JessicaMeixner-NOAA Are all your concerns addressed? I'd like to get this in.

Copy link
Contributor

@JessicaMeixner-NOAA JessicaMeixner-NOAA left a comment

Choose a reason for hiding this comment

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

@WalterKolczynski-NOAA yes, my concerns were corrected. Thanks @JianKuang-NOAA for fixing those.

I do have two questions though: 1. I thought this was waiting for the latest commit of ufs-weather-model which ported things to stampede: ufs-community/ufs-weather-model@37f0da7 and this PR does not update what is checked out to point to that. 2. the modules used in that PR are for hpc stack 1.1.0 which is inconsistent with this.

That being said I have no objection to this going in as it doesn't effect hera/orion/etc.

@WalterKolczynski-NOAA
Copy link
Contributor

Agree, both of those should be addressed. Then this PR can go in.

@JianKuang-Intelsat
Copy link
Contributor Author

@WalterKolczynski-NOAA Could you make these changes? I am focusing on EWOK project now, plus I have no Hera & WCOSS access which means someone other than me need to test it there anyway.

@WalterKolczynski-NOAA
Copy link
Contributor

Since I have to take over, I'm going to merge this PR (since it doesn't break anything), then start from my own clone to update the remaining bits. That is easier than making sure I have all the right permissions to update Jian's branch.

@WalterKolczynski-NOAA WalterKolczynski-NOAA merged commit d2662ca into NOAA-EMC:feature/coupled-crow Mar 25, 2021
@WalterKolczynski-NOAA
Copy link
Contributor

Actually, I just hadn't added Stampede's ssh key to GitHub yet. Oh well, already done. There will be a new PR for the cleanup.

@WalterKolczynski-NOAA
Copy link
Contributor

This PR should've tagged Issue #280

zhanglikate pushed a commit to zhanglikate/global-workflow that referenced this pull request Oct 6, 2022
* If ufs.cpld.cpl.r.* files do not reproduce, try nccmp
* Modify/add to run cpld 35d bmark tests
* Fix nccmp implementation. Fix a bug for 35d tests
* Use env var NCCMP
* Add hera.intel to rt_35 and rt_wave_35d
* Use which to find nccmp path. Apply nccmp to all files that fail cmp. Fix a bug in fv3_ccpp_wrtGauss_netcdf_parallel
* Change compare method from nccmp to compare_ncfile.py
* Netcdf compare changes on Hera; skip-ci
* Netcdf compare changes on WCOSS Dell P3; skip-ci
* Modify comopare_ncfile.py
* Netcdf compare changes on wcoss cray
* Implement Dusan's ecflow fix NOAA-EMC#273
* Move miniconda3 to emc.nemspara on Hera and Orion. Minor change in default_vars.sh
kayeekayee pushed a commit to kayeekayee/global-workflow that referenced this pull request May 30, 2024
…itial, ocn -> wat, merra2 threading (NOAA-EMC#279)

* changed .gitmodules to point to merra2 ccpp/physics
* remove GFDL_atmos_cubed_sphere and ccpp-framework from .git module
* remove IPD gfsphysics
* Update .gitmodules and submodule pointer for ccpp-physics for code review and testing
* Remove interstitial zorl composites
* Update .gitmodules and submodule pointer fpor ccpp-physics for code review and testing
* Remove or replace references to IPD in comments in atmos_model.F90
* Initialize Sfcprop%zorlx to clear_val instead of huge
* Update submodule pointer for ccpp-physics
* Rename Fortran variables and CCPP standard names / long names of surface composites from ocean to water
* Rename Sfcprop%zorlw to Sfcprop%zorlwav
* Rename Sfcprop%zorlo to Sfcprop%zorlw
* update submodule pointer for ccpp-physics
* Revert change to .gitmodules and update submodule pointer for ccpp-physics
Co-authored-by: anning.cheng <anning.cheng@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.

None yet

3 participants