fix: manually inline cap_max_internal in the right computation#11
Merged
CharlesKrop merged 12 commits intoMar 31, 2026
Conversation
The temprory `dzh` is only used in that scope and can be (manually) scalarized. No need to have an IJ-Field.
`cap_max_internal` might be read before written (in case `MOIST_TRIGGER` is not `1`). DaCe wronly scalarizes the variable into both (previous) computation blocks. That leads to a potential read of undefined memory in the later computation block. Manually scoping `cap_max_internal` to the later block ensures that an initial value is written before the variable is read the first time. This is only a temp fix. I'll go debug the DaCe scalarization ...
CharlesKrop
approved these changes
Mar 30, 2026
romanc
force-pushed
the
fix/GF-convective-cloud-base-level
branch
from
88a8f0e to
1f9d4b1
Compare
CharlesKrop
merged commit 1b25938
into
FlorianDeconinck:fdeconinck/feat/pyMoist_infra
8 of 9 checks passed
FlorianDeconinck
added a commit
that referenced
this pull request
Apr 27, 2026
* GF2020: changed PLUME_ORDER to SHALLOW_MID_DEEP, bringing it back in line with fortran behavior * GF2020: removing dynamic intervals * GF2020: updating a couple translate tests that were missed in the DTIME --> DT_MOIST change * GF2020: Updated CumulusParameterization comments with new test results (using dace instead of debug backend) and updated a few comments to reflected changes to translate test names * GF2020: cleaning up large_scale_forcing * better .gitignore (#8) We've been tracking (in git) an empty vscode config and a temp graph from dace. This commit removes both files and - moves the `.vscode/` ignore "up" to the grid comp - adds `_dacegraphs/` to the ignore paths because it will only ever contain invalid/failed dace graphs for debugging. * GF2020: fix typos (#9) * GF2020: fix typos * fix typo in README * GF2020: fix ensemble_forcing_mid_plume (#10) Fix access of `updraft_lfc_level` inside `ensemble_forcing_mid_plume`. * GF2020: updating comments for LargeScaleForcing test * GF2020: removing a quantity --> np array --> stencil, passing a slice of the field to the stencil instead * GF2020: changing .field to .data in a few places * GF2020: .data to .field in translate test * Updated UW to include code for LM=181. This code has not been tested. * constants file was broken when GF2020 was manually merged, fixing * Fixed UW interface and config for 181 level code. And set NUMBER_OF_TRACERS to NCNST. * Clean up of bad code calling down to Python * Protect GetPointer call when association has been read to be False - to avoid `icpx` failure * fix: manually inline `cap_max_internal` in the right computation (#11) * unrelated: fall back to default number of tracers * unrelated: namelist is unused in `TestCore` * unrelated: fixing typos in GF_2020 * unrelated: fixing typos (2) in GF2020 * unrelated: start_level and lcl_level are both 2d * unrelated: unused variables * unrelated: cleanup dzh temporary The temprory `dzh` is only used in that scope and can be (manually) scalarized. No need to have an IJ-Field. * fix: manually inline `cap_max_internal` in the right computation `cap_max_internal` might be read before written (in case `MOIST_TRIGGER` is not `1`). DaCe wronly scalarizes the variable into both (previous) computation blocks. That leads to a potential read of undefined memory in the later computation block. Manually scoping `cap_max_internal` to the later block ensures that an initial value is written before the variable is read the first time. This is only a temp fix. I'll go debug the DaCe scalarization ... * GF2020: fix for DowndraftWindShear * GF2020: fix for DowndraftWindShear * cleanup: don't pass unused arguments --------- Co-authored-by: Charles Kropiewnicki <charles.j.krop@gmail.com> * [dsl]: Fix and cleanup GF2020 and GFDL_1M translate tests (#12) * fix/cleanup GF translate tests This PR fixes and cleans up translate tests of GF2020. The things you will find in this PR are: 1. Fix cumulus parametrization (args of `StaticControl`) 2. Clean up tests by not passing `namelist` down to `TestCore` if it isn't used there. 3. Fix some translate tests (e.g. GF2020) that were initializing `TranslateFortranData2Py` with `namelist`. `namelist` has never been an argument of that base class. 4. Cleaning up tests by not overloading `self.stencil_factory` in the subclass (the baseclass already assigns, given the right arguments are passed to the base class (see 3)). * fix / cleanup GFDL_1M translate tests This commit changes the translate tests of the GFDL_1M microphysics code. In particular, this PR 1. fixes tests that were initiliazing `TranslateFortranData2Py` with a namelist argument (there is no such argument). 2. cleans up translate tests that were shadowing `self.stencil_factory` in the subclasse (already done in the base class). * Updating tracer constants * GF2020: more guards and removing duplicate constant * GF2020 fortran side of the interface * GF2020 python side of the interface * GF2020 integration - python is numerically stable, but does not quite agree with the fortran (consistently about 0.5 degrees C too cold after the first day or so) * GF2020: Safer data dimension management * GF2020: fixed a few small bugs in the numerics * GF2020: put namelist back into a translate test * GF2020: disable two variables in GF2020_CumulusParameterization that fail b/c of a bug on the fortran side * GF2020: minor translate test updates * GF2020: bunch of changes/bugfixes in Setup & Finalize * Minor fix to 181 level code * GF2020: test reorganization renamed one test, moved another to a different folder * GF2020: fixes bringing the code in line with 11.8.1 * GF2020: flipped shallow and deep entrainment in the interface * GF2020: remove seed convection from inputs in GF2020 translate test * GFDL1M: few bugs in the interface * GF2020: stencil in setup needed K_INTERFACE_DIM instead of K_DIM * GF2020: fixing bad fortran conditional * GF2020: added dynamic call support to GF2020 test * GF2020: reworked lat/lon integration to python * GF2020: removing initial precipitable water fields from inputs of GF2020 and GF2020_Setup tests (after they were removed form the savepoint). changed the data source for the GF2020_Finalize savepoint for those two fields * GF2020: getting domain side properly for one of the setup stencils * fixing a dace bug * GF2020: fixed some bad defaults in the interface * GF2020: bugfix in finalize, putting optional outputs in their own stencils, with the execution hidden behind an existance check * GF2020: minor bugfixes in the interface - bad defaults and incorrect alloc=True * Added NOOP to get Fortran-In and -Out files * Added UW init to UW run to get a clean class each timestep * Reset state output vars to zero. Just a test. * Remove debug code. * GF2020: added a pbl level shift to the interface unlike microphysics, gf convection takes a level (pbl_level) from the model state. this level must be adjusted by one to python indexing before the python runs, then back to fortran indexing before it is passed back to the rest of the state * Added flag to turn on/off state recorder. * Formatting fixes from `pre-commit run` and other PR fixes. * GF2020: pre-commit - automatic changes, reformatting and such * GF2020: removing most instances of == True. some cannot be removed, as doing if (X and Y): breaks when x or why is a boolean. Error: "ValueError: Arithmetic expression is not allowed in boolean operation." * GF2020: changing == False to "not" * GF2020: bugfix, changing == True to == 1 for ENABLE_SHALLOW/MID/DEEP, as these fields are integers (albiet always 0 or 1) and not booleans * GF2020: unused imports * GF2020: two minor changes to make pre-commit happy * GF2020: type: ignores and noqas to make pre-commit happy * reverting changes that were made at various times that should not have been pushed * Changed pre-commit line length to 170 * Cast is enforced via dtype callable - which fails lint but is portable between numpy , cupy and python. Type ignored. --------- Co-authored-by: Roman Cattaneo <romanc@users.noreply.github.com> Co-authored-by: Katrina Fandrich <katrina.fandrich@nasa.gov> Co-authored-by: Florian Deconinck <deconinck.florian@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fix is in the last commit, everything else is unrelated cleanup.
The issue I found is the following:
cap_max_internalmight be read before written (in caseMOIST_TRIGGERis not1). DaCe wrongly scalarizes the variable into both (previous) computation blocks. That leads to a potential read of undefined memory in the later computation block. Manually scopingcap_max_internalto the later block ensures that an initial value is written before the variable is read the first time.This is only a temp fix. I'll go debug the DaCe scalarization ...