Skip to content

Conversation

@mathomp4
Copy link
Member

This PR updates io_hdf5.F90 to support HDF5 1.14. Testing showed that between HDF5 1.10 (in current baselibs) and HDF5 1.14 (in GEOSgcm v12 Baselibs) something changed in how you need to open/close/etc. HDF5 files.

In my limited testing (Intel conus and global), this PR seems to be zero-diff with the current Baselibs and as such, can be dribbled in now before updating Baselibs.

I'll keep draft until a full suite test can be made.

Print errcode

Add all the debug from Chat GSFC

Fix fortran

Fix double close

Clean up debug prints

Use v110 libver?

Reorder for comparison ease

Bring back old program

Fix up program

Fixes for types

Remove accidental file

Longer file names

Add debug prints

Try simple open

More tries

Try verbose checkcode

More tests

updates from Chat GPT 5

Remove debug prints
@mathomp4
Copy link
Member Author

@weiyuan-jiang has kindly undertaken a full suite test for this.

@mathomp4
Copy link
Member Author

Well, huh. @weiyuan-jiang did a run for me and:

Full Log File: /discover/nobackup/wjiang/SystemTests/logs/LDAS/2025-08-20/log.12:42:05

Runtype         Clone Build Build Time Model Run/Compare Assim Run/Compare
-------         ----- ----- ---------- ----------------- -----------------
conus           pass  pass  12 min     pass/pass          -- / --
global           --    --    --        pass/pass         pass/pass
globalcs         --    --    --        pass/pass         pass/pass
globalcnclm4     --    --    --        pass/pass          -- / --
debugconus       --   pass   6 min     pass/pass          -- / --
aggconus         --   pass  12 min     pass/FAIL          -- / --
aggglobal        --    --    --        pass/FAIL         pass/FAIL
aggglobalcs      --    --    --        pass/FAIL         pass/FAIL
aggglobalcnclm4  --    --    --        pass/FAIL          -- / --
gnuconus        pass  pass  28 min     pass/pass          -- / --
gnuglobal        --    --    --        pass/pass         pass/pass
gnuglobalcs      --    --    --        pass/pass         pass/pass
gnuglobalcnclm4  --    --    --        pass/pass          -- / --
gnudebugconus    --   pass  17 min     pass/pass          -- / --

The fact that a CONUS run changed makes me think it's something else instead of this PR?

@mathomp4 mathomp4 marked this pull request as ready for review August 25, 2025 14:35
@mathomp4 mathomp4 requested a review from a team as a code owner August 25, 2025 14:35
@gmao-rreichle
Copy link
Collaborator

The fact that a CONUS run changed makes me think it's something else instead of this PR?

@mathomp4 : The non-0-diff results are for "agg". I doubt they matter. I can spot-check to confirm they're the usual roundoff. Was the test run using the new baselines plus the new h5? Or just the new h5?

@mathomp4
Copy link
Member Author

The fact that a CONUS run changed makes me think it's something else instead of this PR?

@mathomp4 : The non-0-diff results are for "agg". I doubt they matter. I can spot-check to confirm they're the usual roundoff. Was the test run using the new baselines plus the new h5? Or just the new h5?

@gmao-rreichle It should just be the hdf5 updates, see GEOS-ESM/GEOSldas#833 which I guess I can close now...or rather should have not opened? I'm new to GEOSldas GC PRs. :)

I am just now in the process of issuing new Baselibs, etc. but that'll be a longer term process since I need to build them up in places.

@gmao-rreichle
Copy link
Collaborator

@mathomp4 : Sorry for the silly question about h5 vs. new baselibs, I just looked it up myself. I'm about to have a few meetings, I'll check the diffs as soon as I can. It's a bit odd that the code changes in io_hdf5.F90 would create non-0-diff results even for "agg", but if the diffs are roundoff (TBC), I think it'll be ok to merge

@mathomp4
Copy link
Member Author

@mathomp4 : Sorry for the silly question about h5 vs. new baselibs, I just looked it up myself. I'm about to have a few meetings, I'll check the diffs as soon as I can. It's a bit odd that the code changes in io_hdf5.F90 would create non-0-diff results even for "agg", but if the diffs are roundoff (TBC), I think it'll be ok to merge

Oh, not silly. I sort of had to go check to make sure myself. I was doing a lot of testing and I really don't want some temporary baselibs getting into a model :)

@gmao-rreichle
Copy link
Collaborator

Mystery solved... The "agg" comparisons fail only for the "ldas_catparam" file. This file contains time-invariant model parameters and is generated in pre-processing. For the most part, the parameters are read from somewhere and immediately written out into the "ldas_catparam" file without any calculations. But one field ("cdcr1") involves a calculation. There's a roundoff diff in this calculation when the test is run on Cascade Lake and the baseline was run on Milan. I'll go ahead and merge the PR.

@gmao-rreichle gmao-rreichle merged commit 094e489 into develop Aug 25, 2025
9 checks passed
@gmao-rreichle gmao-rreichle deleted the feature/update-hdf5-io branch August 25, 2025 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants