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

Add soil temperature and moisture IAU #222

Merged
merged 152 commits into from
Nov 18, 2024

Conversation

tsga
Copy link

@tsga tsga commented Aug 23, 2024

Add Incremental Analysis Update (IAU) to soil temperature and moisture in the NoahMP land model of the UWM's CCPP. Addresses issue: /issues/190

Regression tests for ATM only and coupled (S2S) configurations pass in hera.

Additional land-IAU specific test to be added: control_c48_intel_land_iau (based on the "control_c48_intel" test, with the only difference being the land IAU increment files added to the INPUT directory .

The input.nml is modified to add "land_iau_nml" (do_land_iau=.true. if executing with land IAU)

tsga and others added 30 commits March 16, 2024 16:56
…into feature/lnd_iau

* 'feature/lnd_iau' of https://github.com/tsga/ccpp-physics:
  correct the meta file
  correctly convert from flashes per five minutes to flashes per minute
  In physics/Interstitials/UFS_SCM_NEPTUNE/maximum_hourly_diagnostics.F90, scale lightning threat from flashes per 5 minutes to flashes per minute to match units in metadata
  In physics/Interstitials/UFS_SCM_NEPTUNE/maximum_hourly_diagnostics.meta: change units 'flashes 5 min-1' to 'flashes min-1' and update long name to make clear this is per 5 minutes
* tmp:
  fix arg_table_noahmpdrv_finalize
@grantfirl
Copy link
Collaborator

@tsga Are you finished making changes to this PR?

Copy link
Collaborator

@ClaraDraper-NOAA ClaraDraper-NOAA left a comment

Choose a reason for hiding this comment

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

Science-wise, looks good to me, thanks @tsga

@tsga
Copy link
Author

tsga commented Nov 12, 2024

@tsga Are you finished making changes to this PR?

@grantfirl yes. And the science reviewers are also OK with the changes. The RTs passed (with the -c switch for lnd_iau because its baseline is not on hera). Thank you.

* ufs/dev:
  fix another optional argument in RUC LSM
  fix some more optional variables
  fix some optional arugments
@grantfirl
Copy link
Collaborator

grantfirl commented Nov 14, 2024

@tsga Sorry for the late re-review. Please see the comments above. The only thing that I think must be changed is the addition of the optional = True lines in the metadata. The other comments are to keep other physics developers happy. Also, I understand that this functionality has been under development so there are many lines of code commented here and there as needs changed. Given git history, it should be possible to recover those lines rather than leave them commented out in the code.

I feel comfortable approving once the noahmpdrv.meta has been changed.

@tsga
Copy link
Author

tsga commented Nov 14, 2024

@tsga Sorry for the late re-review. Please see the comments above. The only thing that I think must be changed is the addition of the optional = True lines in the metadata. The other comments are to keep other physics developers happy. Also, I understand that this functionality has been under development so there are many lines of code commented here and there as needs changed. Given git history, it should be possible to recover those lines rather than leave them commented out in the code.

I feel comfortable approving once the noahmpdrv.meta has been changed.

@grantfirl no problem, thank you for the feedback. I will address these soon and progress the PR forward.

@tsga
Copy link
Author

tsga commented Nov 15, 2024

@grantfirl I have now added the optional=True as you suggested, removed all commented lines that are not descriptive comments. I have also removed most of the print statements. Left few that I think are essential but they are now very few and most printed only once.

Copy link
Collaborator

@mkavulich mkavulich left a comment

Choose a reason for hiding this comment

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

Just one question, otherwise looks good

physics/SFC_Models/Land/Noahmp/noahmpdrv.F90 Outdated Show resolved Hide resolved
@FernandoAndrade-NOAA
Copy link

Testing for 2415 has completed, please continue with the merge process, thank you.

@grantfirl
Copy link
Collaborator

@tsga @mkavulich It looks like the last commit just got in under the deadline for final UWM tests, so I'm going to go ahead and merge this.

@grantfirl grantfirl merged commit cfa1861 into ufs-community:ufs/dev Nov 18, 2024
@tsga
Copy link
Author

tsga commented Nov 18, 2024

@tsga @mkavulich It looks like the last commit just got in under the deadline for final UWM tests, so I'm going to go ahead and merge this.

Thank you so much @grantfirl and everyone for guiding me through these PR. Cheers.

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.

10 participants