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

Open
wants to merge 105 commits into
base: ufs/dev
Choose a base branch
from

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 turning on of land IAU and soil temperature increment files inside INPUT directory). The test inputs are at:
/scratch1/NCEPDEV/da/Tseganeh.Gichamo/ccpp-physics/test/control_c48_intel_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
@tsga tsga marked this pull request as ready for review September 16, 2024 12:31
@tsga
Copy link
Author

tsga commented Sep 16, 2024

Can't assign so tagging relevant people here. Please add (tag) anyone you think should be aware.
@CoryMartin-NOAA, @barlage, @ClaraDraper-NOAA, @CatherineThomas-NOAA, @junwang-noaa

@grantfirl
Copy link
Collaborator

@tsga 2 of the 5 folks that you've requested to be added as reviewers have been added. The other 3 (@CoryMartin-NOAA, @ClaraDraper-NOAA, @CatherineThomas-NOAA) need to accept an invitation that I just sent to be collaborators on this repo before they can be added as reviewers.

Is there specific urgency for this work to be added to the UFS merge queue? We're trying to prioritize code reviews for a couple of "heavy" PRs.

@ClaraDraper-NOAA
Copy link
Collaborator

@grantfirl Tseganeh is on annual leave, so I'll jump in. This PR is needed for testing of the land DA for GFSv17, which we hope to start in a few weeks (but could use a branch initially, if we must). It will need to be merged before we test the land DA in the formal DA prototypes (not possible until we have ensembles in the prototype).

! time step informatoin, etc)
type (land_iau_control_type) :: Land_IAU_Control
! Land IAU Data holds spatially and temporally interpolated soil temperature increments per time step
type (land_iau_external_data_type) :: Land_IAU_Data !(number of blocks):each proc holds nblks
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dustinswales Could you help me to understand whether this will cause an issue similar to #74?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@grantfirl I think it will. The solution for the photochemical schemes was to bundle everything up into a type and pass it in, so that each instance had a copy. But those are tiny schemes.

Another solution is to move its declaration to the module where it's defined, and set it up during initialization. For example, see how the RRTMGP longwave driver sets up the "scheme native" gas-optics DDT. This approach should work as well, but keep in mind that the GP initialization routines are embedded with MPI commands, which aren't present in these type initializations. Also, this assumes that the data is the same across all instances, which it is for GP.

Unfortunately we don't have a way to test this, so we should at some point brainstorm a way to build a way to test this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

^This is straightforward, but requires some effort.

! time step informatoin, etc)
type (land_iau_control_type) :: Land_IAU_Control
! Land IAU Data holds spatially and temporally interpolated soil temperature increments per time step
type (land_iau_external_data_type) :: Land_IAU_Data !(number of blocks):each proc holds nblks
Copy link
Collaborator

Choose a reason for hiding this comment

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

@grantfirl I think it will. The solution for the photochemical schemes was to bundle everything up into a type and pass it in, so that each instance had a copy. But those are tiny schemes.

Another solution is to move its declaration to the module where it's defined, and set it up during initialization. For example, see how the RRTMGP longwave driver sets up the "scheme native" gas-optics DDT. This approach should work as well, but keep in mind that the GP initialization routines are embedded with MPI commands, which aren't present in these type initializations. Also, this assumes that the data is the same across all instances, which it is for GP.

Unfortunately we don't have a way to test this, so we should at some point brainstorm a way to build a way to test this.

! time step informatoin, etc)
type (land_iau_control_type) :: Land_IAU_Control
! Land IAU Data holds spatially and temporally interpolated soil temperature increments per time step
type (land_iau_external_data_type) :: Land_IAU_Data !(number of blocks):each proc holds nblks
Copy link
Collaborator

Choose a reason for hiding this comment

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

^This is straightforward, but requires some effort.

physics/SFC_Models/Land/Noahmp/noahmpdrv.F90 Outdated Show resolved Hide resolved
remove empty subroutine noahmpdrv_timestep_finalize
remove empty subroutine: noahmpdrv_timestep_finalize
removed pointer attribute from "input_nml_file" declaration
remove commented out old lines
get rid unused var input_nml_length
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.

4 participants