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

Updates for component NoahMP land model #2387

Open
wants to merge 59 commits into
base: develop
Choose a base branch
from

Conversation

uturuncoglu
Copy link
Collaborator

@uturuncoglu uturuncoglu commented Aug 2, 2024

Commit Queue Requirements:

  • Fill out all sections of this template.
  • All sub component pull requests have been reviewed by their code managers.
  • Run the full Intel+GNU RT suite (compared to current baselines) on either Hera/Derecho/Hercules
  • Commit 'test_changes.list' from previous step

Description:

The PR aims to bring recent updates from component land model. The major changes are,

  • Add new decomposition algorithm to NoahMP component model NUOPC cap to distribute land and ocean points evenly to all the processor. The new decomposition algorithm might be more efficient especially for high resolution cases. The new decomposition algorithm can be activated by adding decomp_type = custom option to LND_attributes:: section of ufs.configure.
  • A new flux calculation method is implemented in CCPP/Physics sfc_land scheme to overcome initialization issues. We also adjust run sequences of land related configurations.
  • New RTs based on S2S configurations are introduced and they include extra component as land. The new tests called as cpld_control_p8_lnd and control_restart_p8_atmlnd.
  • The atm-lnd side-by-side RT (control_p8_atmlnd is removed since CCPP/Physics version of the code diverged from the component model. Some of the information provided by the CCPP/Physics can not be provided in the component model. For example, update for land IAU in the ccpp-physics NoahMP. The side-by-side run can be still performed by running control_p8_atmlnd or cpld_control_p8_lnd by setting cpllnd2atm = .false.. This will turn off feedback from land component to atmospheric model and allow atmospheric model receive land fluxes from CCPP/Physics version of NoahMP.
  • The issues in atm-lnd configuration related with the inconsistent input files are solved by using input files form coupled configuration.

Commit Message:

* UFSWM - 
  * AQM - 
  * CDEPS - 
  * CICE - 
  * CMEPS - 
  * CMakeModules - 
  * FV3 - 
    * ccpp-physics - 
    * atmos_cubed_sphere - 
  * GOCART - 
  * HYCOM - 
  * MOM6 - 
  * NOAHMP -
  * WW3 - 
  * stochastic_physics - 

Priority:

  • Critical Bugfix: Reason
  • High: Reason
  • Normal

Git Tracking

UFSWM:

Sub component Pull Requests:

UFSWM Blocking Dependencies:

  • Blocked by #
  • None

Changes

Regression Test Changes (Please commit test_changes.list):

  • PR Adds New Tests/Baselines.
    Two new RTs are added: cpld_control_p8_lnd and control_restart_p8_atmlnd
    One RT is removed: control_p8_atmlnd_sbs
  • PR Updates/Changes Baselines.
    All land related RTs have answer changes due to the changes in the input files and also change the way of model initialization.

Input data Changes:

  • None.

Library Changes/Upgrades:

  • Required
    • Library names w/versions:
    • Git Stack Issue (JCSDA/spack-stack#)
  • No Updates

Testing Log:

  • RDHPCS
    • Hera
    • Orion
    • Hercules
    • Jet
    • GaeaC5
    • GaeaC6
    • Derecho
  • WCOSS2
    • Dogwood/Cactus
    • Acorn
  • CI
  • opnReqTest (complete task if unnecessary)

@uturuncoglu
Copy link
Collaborator Author

@junwang-noaa I created a draft PR to bring recent updates from land side and I'll keep working on it. Once it is ready, I'll change it's status. Probably, this would be the last update from land JTTI project since we are plaining to finalize it around end of August.

@DeniseWorthen
Copy link
Collaborator

@uturuncoglu Does this PR require an ESMF update for the "stripes" fix?

@uturuncoglu
Copy link
Collaborator Author

@DeniseWorthen No, I put a fix in CMEPS side, now it uses redist rather then interpolation (see the CMEPS PR for more detail). Since we are using same grid in both land and atmosphere that works for the RTs. CMEPS is assuming they are using same grid by default. Once we want to run land in a tiger resolution under S2S configuration, then we need newer version of ESMF that has fix.

@DeniseWorthen
Copy link
Collaborator

Thanks for the explanation. What about LM4?

@uturuncoglu
Copy link
Collaborator Author

@DeniseWorthen LM4 has no fully coupled configuration yet and it is only coupled with DATM. So, it won't affect it. But, I still need to add samegrid_atmlnd=.false. to also those tests since mediator mainly assumes that atmosphere and land grids are same which is not correct for data atmosphere could configurations.

@uturuncoglu
Copy link
Collaborator Author

@JustinPerket @barlage I am seeing following message in the error file "18: FATAL from PE 18: soil_mod: soil_step_2: conservation of Water is violated; before= 4020.126371497007 after= 0.000000000000000 diff= -4020.126371497007 at lon= 138.8500 lat= -16.3099 i= 67 j= 17 tile= 1 face= 4 time=2000-01-01 00:00:00". this could be related with the new initialization routine. Let me quickly make it optional. So, that could fix the LM4 stability issues. BTW, it would be nice to enable testing on Hercules too. Let me know what you think? @jkbk2004 The fix will come soon. I'll update you about it.

@barlage
Copy link
Collaborator

barlage commented Jan 23, 2025

@uturuncoglu @JustinPerket I'm a little confused at why this is affecting the LM4 simulations. Aren't all the LM4 ICs coming in from LM4-specific inputs, i.e., not using inputs from the atmosphere?

@JustinPerket
Copy link
Contributor

@jkbk2004 @JustinPerket let me check. It could change an answers due to initialization but I am not expecting failure and Hercules tests that I did was fine. I’ll double check and try to fix it soon.

I can also check out your branch and see what's going on in LM4.

@JustinPerket @jkbk2004 this particular test is only available on hera orion gaeac5. Why Hercules is not listed in here. Is there any limitation in terms of inputs. This explains why it did not failed in my end since it is not run on Hercules. I have no access any of those machines but will try to update rt.conf and try to run on Hercules.

That was the case before the datm LM4 PR (#2146) went in. Now LM4 inputdata should be propagated. The PR did pass on other machines, including Hercules. I only have access to hera orion and gaeac5, so I can't confirm. But I think the + hera orion gaea requirement for the datm_cdeps_lm4 test can be removed.

@uturuncoglu
Copy link
Collaborator Author

@JustinPerket Okay. I have a fix in place in sfc_land and testing now. I think LM4 test changes can be done later with your additional PR. I don't want to change too much in the same time. Anyway, I'll update you soon about the fix.

@uturuncoglu
Copy link
Collaborator Author

@jkbk2004 @JustinPerket Okay. I found the issue. samegrid_atmlnd = .false. needs to be added to the data atmosphere coupled configurations. With this it is working. I could not catch this before since lm4 tests was not running on Hercules. @jkbk2004 I think it is ready for testing again (BTW, I could not run lm4 tests on Orion even with develop - I think there is some issue in my environment).

@JustinPerket
Copy link
Contributor

oh, yes, I did have to do that as well in my in-progress coupled atm / LM4 branch, for datm_cdeps_lm4_gswp3_rst
and datm_cdeps_lm4_gswp3.

@uturuncoglu
Copy link
Collaborator Author

@JustinPerket I introduced that parameter due to issue in ESMF (was creating stripes in FV3 cubed spare - but fixed on lates release) and the way of CMEPS structured. The CESM checks the name of atm and land meshes and set flag to indicate grids are same or not but under UFS WM it is not working since we are not using ESMF mesh files. The only way to indicate meshes are same is to pass this new argument. Anyway, I hope this will work. The tests on Hercules are working fine nut I could not check on Orion.

@JustinPerket
Copy link
Contributor

Makes sense. I just checked just to be sure, and your new commit passes LM4 datm RTs on orion (I also have to tweak rt.sh to run on that machine)

@uturuncoglu
Copy link
Collaborator Author

@JustinPerket I just test in my end and both configuration is running on Hercules but I did not change the rt.conf to enable them in my PR since they don't have baseline on Hercules too. I could not test on Orion to check agains baseline since my account is not working properly there but I am not expecting any issue. If you could test quickly under your account we could be sure. If you want you could clone it with git clone -b feature/land_update --recursive https://github.com/uturuncoglu/ufs-weather-model.git.

@jkbk2004
Copy link
Collaborator

@uturuncoglu can you clean up tests/tests/cpld_restart_p8_lnd as well? I don't think you need ufs.cpld.ww3.r.2021-03-23-21600.nc

@JustinPerket
Copy link
Contributor

@JustinPerket I just test in my end and both configuration is running on Hercules but I did not change the rt.conf to enable them in my PR since they don't have baseline on Hercules too. I could not test on Orion to check agains baseline since my account is not working properly there but I am not expecting any issue. If you could test quickly under your account we could be sure. If you want you could clone it with git clone -b feature/land_update --recursive https://github.com/uturuncoglu/ufs-weather-model.git.

Can confirm. Earlier I tested your latest commit 08f3fe69, datm_cdeps_lm4_intel and datm_cdeps_lm4_gswp3_rst_intel passed

@uturuncoglu
Copy link
Collaborator Author

@JustinPerket That is perfect. Thanks for your help.

@uturuncoglu
Copy link
Collaborator Author

@jkbk2004 Okay. Let me fix it too.

@uturuncoglu
Copy link
Collaborator Author

@jkbk2004 Okay. I removed but did not push. Do you want me to push now? I could not be sure since workflow is running and push might trigger a fresh one.

@jkbk2004
Copy link
Collaborator

@jkbk2004 Okay. I removed but did not push. Do you want me to push now? I could not be sure since workflow is running and push might trigger a fresh one.

@uturuncoglu should be ok to push.

@uturuncoglu
Copy link
Collaborator Author

@jkbk2004 done.

@jkbk2004
Copy link
Collaborator

@FernandoAndrade-NOAA @BrianCurtis-NOAA My test on orion ran ok. I will push the log in an hour or so. This pr is ready.

@jkbk2004 jkbk2004 added the Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked. label Jan 23, 2025
@jkbk2004 jkbk2004 removed the jenkins-ort run ORT testing label Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Baseline Updates Current baselines will be updated. Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Atm-Lnd tests are not using V2 surface files
9 participants