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

Update to MAPL v2.44.1 to fix "binary + bit shaving" bug #734

Merged
merged 5 commits into from
Mar 20, 2024

Conversation

mathomp4
Copy link
Member

@mathomp4 mathomp4 commented Mar 18, 2024

This PR updates GEOSldas to MAPL v2.44.1 (see GEOS-ESM/MAPL#2658). This fixes a MAPL bug that was introduced in MAPL v2.8.6.

In MAPL v2.8.6 (used in GEOSldas v17.9.6), a bit-shaving option was added for MAPL binary output. Unfortunately, the bit shaving was done in place for binary, instantaneous output. Depending on the output variables, the bit shaving degraded the internal state (or model prognostic variables) and thereby adversely impacted the simulation and the checkpoint/restart files.

GEOSldas routinely uses MAPL binary output to write tile-space data, which are then converted to netcdf in post-processing using the GEOSldas utility tile_bin2nc4.F90. So the MAPL bug could impact GEOSldas simulations even when the final output files are nc4 format.

The SMAP L4 ops system does bit-shaving in post-processing, so the L4_SM products are ok. Science runs using v17.9.6 to v17.13.1 and recent branches/tags are impacted depending on the HISTORY specs.

@mathomp4
Copy link
Member Author

Note: Labeled as non-zero-diff, but only because that's a guess. Maybe it is?

@biljanaorescanin
Copy link
Contributor

With this PR adding instantaneous 6h collection to GLOBAL/model test doesn't affect the restart file.
I will work with @mathomp4 to add one "inst" bit shaved collection to nightly tests so we cover that option during regular testing.

@biljanaorescanin
Copy link
Contributor

biljanaorescanin commented Mar 19, 2024

@gmao-rreichle @mathomp4 @weiyuan-jiang @bena-nasa After running nightly tests we do have 2 failed model run tests: debugconus and gnudebugconus.
In both failed test runs error is the same just one test has more info.
error from debugconus:
forrtl: severe (408): fort: (3): Subscript #1 of the array DOIT has value -1 which is less than the lower bound of 1

Image PC Routine Line Source
libMAPL.base.so 00002B36FD528A78 biniomod_mp_mapl_ 1988 BinIO.F90
libMAPL.history.s 00002B36FAC560FA mapl_historygridc 3641 MAPL_HistoryGridComp.F90

error from gnudebugconus :

/bin/rm: cannot remove 'strip': No such file or directory
At line 1988 of file /discover/nobackup/borescan/SystemTests/builds/LDAS_GNUCONUS/CURRENT/GEOSldas/src/Shared/@MAPL/base/BinIO.F90
Fortran runtime error: Index '-1' of dimension 1 of array 'doit' below lower bound of 1

Here:
https://github.com/GEOS-ESM/MAPL/blob/b09ae3085e8096c2fcc65f4ca6c0444226955477/base/BinIO.F90#L1988

@mathomp4 mathomp4 mentioned this pull request Mar 19, 2024
7 tasks
@gmao-rreichle
Copy link
Contributor

Thanks, @biljanaorescanin, for running the tests. @bena-nasa: Does the error message reported by @biljanaorescanin above make any sense to you?

@weiyuan-jiang
Copy link
Contributor

It seems a bug. Above that line, a line if(i == -1) cycle should be added. @bena-nasa

@biljanaorescanin
Copy link
Contributor

All nightly tests passed.
PR was also tested for its functionality, adding "inst" collection didn't change the restart file.

@mathomp4 mathomp4 marked this pull request as ready for review March 20, 2024 13:52
@mathomp4 mathomp4 requested a review from a team as a code owner March 20, 2024 13:52
@mathomp4
Copy link
Member Author

@biljanaorescanin I've updated the PR with the new MAPL 2.44.1 release.

@gmao-rreichle gmao-rreichle changed the title Fix binary + bit shaving bug Update to MAPL v2.44.1 to fix "binary + bit shaving" bug Mar 20, 2024
@gmao-rreichle gmao-rreichle merged commit 4afd1f6 into develop Mar 20, 2024
7 checks passed
@gmao-rreichle gmao-rreichle deleted the bugfix/mathomp4/ben-mapl-fix branch March 20, 2024 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants