ufs-community / ccpp-physics

UFS fork for CCPP
Other
4 stars 33 forks source link

Refactor NRL h2o photochemical physics scheme #213

Closed dustinswales closed 1 month ago

dustinswales commented 3 months ago

Included in this PR are changes to NRL's stratospheric h2o photochemistry scheme to make it "safe across multiple CCPP instances". These changes are identical to the reorganization that o3 photochemistry scheme underwent in the fall of 2023..

Both schemes are called from a new module, GFS_photochemistry, at the start of the time-split section of the suite. Previously, the ozone scheme was called from GFS_stateout_update, and the h2o scheme was called just after.

Question @grantfirl @climbfuji The o3 and h2o photochemistry schemes are both "time-splitting". Maybe this is a good place to start with the cleanup for MPAS effort (e.g. enforcing the procces-splitting requirement)?

dustinswales commented 3 months ago

No rush on this.

climbfuji commented 3 months ago

Question @grantfirl @climbfuji The o3 and h2o photochemistry schemes are both "time-splitting". Maybe this is a good place to start with the cleanup for MPAS effort (e.g. enforcing the proccess-splitting requirement)?

I would not try to attempt commingling the changes from @michakales and framework-related development.

With this PR and the previous PRs for ccpp-framework and ccpp-physics, are all changes from the original PR included? Or is there still something missing?

dustinswales commented 3 months ago

Question @grantfirl @climbfuji The o3 and h2o photochemistry schemes are both "time-splitting". Maybe this is a good place to start with the cleanup for MPAS effort (e.g. enforcing the proccess-splitting requirement)?

I would not try to attempt commingling the changes from @michakales and framework-related development.

With this PR and the previous PRs for ccpp-framework and ccpp-physics, are all changes from the original PR included? Or is there still something missing?

@climbfuji The only piece missing from the OG PR is to move the Thompson mp "is_initialized" flag from module memory to GFS_typedefs. (This will avoid needing to bring in the "instance" field into the physics schemes.

grantfirl commented 2 months ago

@dustinswales Looks like there are some comments that need to be addressed before this is put in the queue?

dustinswales commented 2 months ago

@grantfirl Yeah. I've been meaning to circle back to this. I will update and address the comments later this afternoon.

grantfirl commented 2 months ago

@climbfuji Please re-review and see if https://github.com/ufs-community/ccpp-physics/pull/213/commits/420317e6b8e4adb57c68b1ea9381d1a15f4cc86f addresses your concerns.

grantfirl commented 1 month ago

@dustinswales I'm thinking of combining this with https://github.com/ufs-community/ccpp-physics/pull/218 to expedite testing/merging since this doesn't chance baselines. Is that OK with you?

grantfirl commented 1 month ago

@dustinswales Combined into https://github.com/ufs-community/ccpp-physics/pull/223 (Scheduled for UFS merge queue early next week)