Conversation
mathomp4
added
0 diff
gmao-rreichle
left a comment
gmao-rreichle
left a comment
There was a problem hiding this comment.
@mathomp4 : Thanks for putting together this PR and checking in about any land impact.
The only mods I see in the rasterization code are in mkMITAquaRaster.F90, which is for the coupled model and about which I know nothing. To be safe, @biljanaorescanin can test the creation of coupled-model bcs.
I see some changes in mk_restarts, which all look innocuous. To be safe, @biljanaorescanin can test the remap_restarts package. Note that some of the mk_restart files that are touched in this PR may be slated for removal. We may (or may not) still be waiting for the v12 GCM to make it to develop so we can clean up old code that goes with "regrid.pl", which is no longer supported in the v12 GCM. And I can never remember which mk_restart programs are used only by "regrid.pl" (but not by the new "remap_restart" python package). @weiyuan-jiang would know.
mathomp4
removed
the
Non 0-diff
|
@gmao-rreichle I can say this is zero-diff to stock v12 as of today as far as the model is concerned. So I believe using I'll try remap_restarts now as well sparing @biljanaorescanin from needing to do that. :) |
Thanks, @mathomp4. Just in case the info got lost, we have a set of tests for remap_restarts: |
Update. I did a remap from C180 CS Ocean v12 Land to C48 MERRA2 Ocean NL3 Land and both this PR and stock v12 are identical. I think we are good |
Oh yeah. @weiyuan-jiang recently fixed that for us. Let me try that... |
|
@gmao-rreichle So, huh. Exactly one restart is not identical. The
Per internet this number: 1.401298e-45 is "exactly one float32 ULP at the smallest denormal, meaning some of these differences are literally 1 bit in the mantissa". So I think this means there was a slight rounding difference somewhere. |
|
Per a suggestion from @tclune I'm doing a test now where I undo this change and run the tester again. If those are identical, then the difference is something between the v12 @weiyuan-jiang used for the Baselines (which is more of a "stock v12" than the "development v12" I started from) |
This is confirmed. The difference is due to something that changed in v12 not in my PR. Phew. Now what was the change? I dunno, but I didn't do it! 😄 |
mathomp4
removed
the
Non 0-diff
|
I've removed the non-zero-diff label as this seems to be very zero-diff! Huzzah! |
@mathomp4 @biljanaorescanin @weiyuan-jiang : Do we know if the non-zero-diff might also apply to "develop"? Matt tested this on the "v12" branch, but that doesn't mean it's not present in develop (v11). We don't always test for zero-diff of remap_restarts, and such a non-zero-diff change in remap_restarts might well have slipped through the cracks on develop. |
|
Last week when I tested develop |
@biljanaorescanin : Are you talking about the routine nightly tests here or about testing remap_restarts? |
Running test_remap_restarts.py package from what you get with develop GCM v11. |
Yes. At the moment the test script will target v12. If needed, we could create a v11 baseline for v11 testing? |
|
The difference is only on fvcore and related restarts like moist. It is zero-diff for catch, lake, landice. @gmao-rreichle |
@mathomp4 , @sdrabenh : I approved the PR, based on Matt's finding that it's zero-diff. We will have to follow up, though, on the non-zero diff results for remap_restarts that happened before this PR. @weiyuan-jiang , @biljanaorescanin : I'm getting a bit lost what we are and aren't seeing here in terms of non-zero diff results. It doesn't really matter that the non-zero-diff results are only in restarts related to fvcore and moist. Remap_restarts as a whole must be zero-diff, not just catch-related restarts. If there are any non-zero diff results, we need to explain, evaluate, and document the non-zero-diff change. Let's discuss at our next LDAS tagup. |
4 participants
This PR has fixes for Flang 22
NOTE: I am keeping this draft for now. I'm going to test if this is zero-diff for v12. It should be, but I want to be safe.
There is a small chance that my usingIt isnint()for the access-array-with-reals could be different if ifort does anint().int()which what the compilers do.I'll also probably want to consult with @biljanaorescanin and @gmao-rreichle as flang found issues in the rasterizing code. Again mostly access-arrays-with-reals. → Testing with remap_restarts shows it is zero-diff here as well. Nice.
This pull request primarily focuses on improving type safety and compatibility in several Fortran routines across the GEOS physics grid components. The most significant changes are the consistent application of
int()to ensure integer indices, enhancements to file handling for robustness, and improved compiler compatibility checks. Below are the most important changes grouped by theme:Type Safety and Indexing Improvements:
int()in multiple routines, including wind and stress calculations ingw_convect.F90, vegetation tile handling in carbon regridding routines, and turbulence height assignment, ensuring indices are always integers and preventing potential runtime errors. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]File Handling Robustness:
mkMITAquaRaster.F90from manualfstatcalls to Fortran'sinquirestatement with error checking, improving reliability and error reporting when opening grid files. Also changed file size variables to useINT64for large file support. [1] [2] [3] [4]Compiler Compatibility Enhancements:
ftellexternal declaration to support bothgfortranandflang, increasing portability across compilers in several restart and utility programs. [1] [2] [3] [4] [5] [6]Minor Codebase Cleanups:
FMS::fmsfrom theGEOSmoist_GridCompCMake configuration, streamlining build dependencies.iargcincv_SaltRestart.F90, resolving a potential undeclared variable issue.These changes collectively improve the reliability, maintainability, and portability of the GEOS physics grid component codebase.