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

[NDSL] GFDL Single moment microphysics port to NDSL #1041

Open
wants to merge 20 commits into
base: dsl/develop
Choose a base branch
from

Conversation

CharlesKrop
Copy link

@CharlesKrop CharlesKrop commented Dec 30, 2024

Summary

GFDL single moment microphysics port to NDSL of the driver, corresponding to gfdl_cloud_microphys_driver in the source Fortran.

  • Port all code for FP configuration
  • Partial validation (see below) on one-step numerical regression with dace:cpu backend
  • Integration into GEOS

Partial Validation

GFDL_1M_driver created. Primary components: fall_speed, terminal_fall, warm_rain, icloud. All components are considered correct on their own, with minor errors (order 10^-9/10^-10 or <100 ULP in most cases) in warm_rain and icloud.

Putting these components together results in divergence in nearly all variables. Most are minor, but a few (e.g PRCP_RAIN ) are significant. These errors are most likely the accumulation of the small errors from the aforementioned driver components as the driver runs its iterations (in the test case, these functions were called 12 times).

Configuration ported

During construction numerous unexecuted code paths were discovered, controlled by boolean keywords. This keyword control pattern has been replicated, but unexecuted code was not carried over. Instead, if one of these paths would be triggered, the keywords will prevent execution.

Written but unverified: warm_rain, i_cloud, iqs1, iqs2, wqs1

Written, verified, but incorrect: all tables. Errors are small (< 100 ULP) for tables. des tables (which are just the gradients of the tables themselves) have larger errors, but none of these errors cause significant problems in later calculations, so I am putting them aside for now.
…if, before major else). warm rain does not verify. only fails because of implicit rain "function" call. this "function" runs perfectly within the terminal_fall port (called three times with different data of similar order of magnitude), but fails within warm_rain
…ic, DQVDTmic, DQLDTmic, DQIDTmic, PRCP_ICE, PFI_LS

DQIDTmic and DUDTmic errors are small in magnitude not tremendous concern. Others are a problem
…heck_flags error handling capabilities.

Driver passes when testing only the first loop (with the exception of DQADTmic), errors grow with each subsequent loop, compounding to some pretty serious errors after the final iteration. Cause unknown
@CharlesKrop CharlesKrop self-assigned this Dec 30, 2024
@CharlesKrop CharlesKrop requested review from a team as code owners December 30, 2024 19:07
@FlorianDeconinck FlorianDeconinck changed the title GFDL_1M_driver [NDSL] GFDL_1M_driver Dec 30, 2024
@FlorianDeconinck FlorianDeconinck added the 0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.) label Dec 30, 2024
@FlorianDeconinck FlorianDeconinck changed the title [NDSL] GFDL_1M_driver [NDSL] GFDL Single moment driver port to NDSL Dec 30, 2024
Copy link
Collaborator

@FlorianDeconinck FlorianDeconinck left a comment

Choose a reason for hiding this comment

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

First look. More docstrings on all functions and rename of the generic "loop" functions + inline coments

.DS_Store Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do not commit

Copy link
Author

Choose a reason for hiding this comment

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

I don't see these files in my vs code to remove them

self.melting_mask_2 = quantity_factory.zeros(
[X_DIM, Y_DIM, Z_DIM], "n/a", dtype=bool
)
self.current_k_level = quantity_factory.zeros(
Copy link
Collaborator

Choose a reason for hiding this comment

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

THIS_K?

Copy link
Author

Choose a reason for hiding this comment

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

more complicated than that. this is the double k loop we talked about briefly. its not executed in the test case, but I fear we will run into a case where this is turned on pretty quickly, so I'm leaving the code there. it works when I force it to execute, but I obviously can't validate it.

THIS_K sort of solves the problem, but not entirely. Katrina and I spent a while brainstorming this and this was the best (albeit still not good) solution we could come up with that actually runs. if/when we need to tackle this, we can talk and figure out something better

@@ -47,6 +47,7 @@ def QSat_Float_Liquid(
PL: Float = -999.0,
DQ_trigger: bool = False,
):
DQ = 0.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a qsat fix? If so it needs to be PR'ed separately since it's shared code

export PACE_TEST_N_THRESHOLD_SAMPLES=0
export PACE_DACE_DEBUG=True
export GT4PY_COMPILE_OPT_LEVEL=0
python -m pytest -s \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Revert from line 11 downwards

@FlorianDeconinck FlorianDeconinck changed the title [NDSL] GFDL Single moment driver port to NDSL [NDSL] GFDL Single moment microphysics port to NDSL Dec 30, 2024
FlorianDeconinck and others added 5 commits December 30, 2024 17:28
…er port

Fix all string `cffi.FFI.CDATA` with __annotations__ since py 3.11
…o a class (namelist_constants)

moved length constant to driver_constants
@CharlesKrop CharlesKrop requested a review from a team as a code owner January 8, 2025 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff trivial The changes in this pull request are trivially zero-diff (documentation, build failure, &c.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants