fixes #2657

[bena-nasa]

Description

Related Issue

As the title says this fixes #2657
If the the binary output is bit shaved, I copy to a new state before writing.

In reality this is only necessary for instantaneous output but I was trying to avoid a whole slew of conditions and it doesn't matter in the end if we make a few extra copies

I've marked this as zero-diff because is is zero-diff for the GEOSgcm.x although it is not for the LDAS.

Motivation and Context

How Has This Been Tested?

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Trivial change (affects only documentation or cleanup)

Checklist:

• I have tested this change with a run of GEOSgcm (if non-trivial)
• I have added one of the required labels (0 diff, 0 diff trivial, 0 diff structural, non 0-diff)
• I have updated the CHANGELOG.md accordingly following the style of Keep a Changelog

[tclune]

@bena-nasa I think this needs to be changed to nonzero diff. If I understood correctly it really can change answers.

Also ... does this possibly impact the results that the modeling team reported yesterday? Were we shaving one of their fields or only a diagnostic?

[mathomp4]

Converting to draft. Per @biljanaorescanin the debug runs of the LDAS are throwing a failure:

GEOS-ESM/GEOSldas#734 (comment)

Intel:

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

GNU:

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

I suppose the "good" thing is it's the same error it seems.

[mathomp4]

@bena-nasa I think this needs to be changed to nonzero diff. If I understood correctly it really can change answers.

Also ... does this possibly impact the results that the modeling team reported yesterday? Were we shaving one of their fields or only a diagnostic?

@tclune I don't think anyone in the AGCM world operates in such a way to trigger this. But I'll work with @bena-nasa to get specifics on when this could be triggered so we can let people know in the release notes and changelog.

[bena-nasa]

@biljanaorescanin
Matt showed me the error you were seeing in the debug run. I will look into this. Hang tight

[mathomp4]

The CI failure looks to be some weird issue on their side. I'm re-launching

[biljanaorescanin]

With last commit all my land tests passed.

[bena-nasa]

I did some spot checking of the output files by using that converter, made sure that the shaved vs non-shaved output was right in that each variable was "right" in that they were basically the same modulo the bit shaving. I.E. I didn't do something like somehow write the variables in the wrong order in the binary file in say the shaved path.
So yeah, sounds like good to go