Fixes #1199 - HorizontalFluxRegridder numerical bug.

[tclune]

Description

Fixing numerical bug in HorizontalFluxRegridder

Related Issue

Motivation and Context

bugfix

How Has This Been Tested?

compiles on Intel
is not exercised by GEOS

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]

Here are the results of regridding "pure" values of 2 for x and y fluxes:

output varies from 1.9996 to 2.041. Before it was identically 4.

[tclune]

@sdeastham Careful interpreting the above results. Because the input fluxes were constant, the regrid essentially returns

y = (d_1 * x_1 + d_2 *x_2) / (d_12) =  (d_1 + d_2)*x/(d_12).   

I.e., it just measures how accurate the formulae for the arc lengths, as mathematically d_12 = d_1 + d_2

[tclune]

Errors appear mostly near the poles. I think that is because "longitude" is very sensitive to position in that vicinity and increases error in the trig expressions.

[sdeastham]

Thanks @tclune ! So, to clarify, what's being plotted is the error in the new calculation? Something I'm curious about - is the calculation of length identical to the method used in FV3? My thinking is that, if so, then this error shouldn't actually matter (because the same error was present in FV3 when calculating the fluxes).

[tclune]

No the output fields from a test signal are being output. The variation from "2" is the error (entirely due to finite precisions FP, not to regridding).

@bena-nasa has set up a new test that has spatially varying input which will be a better indicator. But harder to interpret. :-) Hopefully I can exercise that next week.

[sdeastham]

Ha sorry, that's what I meant - so if x is the plotted value, the relative error is x/2 - 1. Do you know though if this error would actually affect the output? If I understand you and the code correctly, the calculation proceeds as follows (see caveat underneath regarding distance calculation):

• At C180, FV3 calculates mass fluxes across two faces. To figure out the flux per unit face length two adjacent cells F1 and F2, it uses lengths d1 and d2.
• When we reuse that data at C180, that's fine - we have a completely consistent understanding of the arc lengths, whether or not those lengths are strictly correct. FV3 calculates the total flux as F1 * d1 and F2 * d2.
• When we reuse the data at a coarser resolution (say, C90), the regridder says that the total flux per unit length is F12 = ((F1 * d1) + (F2 * d2))/(d12). FV3 doesn't know anything about the finer grid so it calculates the total flux across the face as F12 * d12, so the error e is e = F12 * d12 - ((F1 * d1) + (F2 * d2)) = ((F1 * d1) + (F2 * d2)) * d12/d12 - ((F1 * d1) + (F2 * d2)) = 0. So even if d12 is not quite the same as d1 and d2... I think?

The above wouldn't be true if the regridder calculated the coarse flux as F12 = ((F1 * d1) + (F2 * d2))/(d1 + d2), but as long as it uses d12 in the denominator, I think this error doesn't affect the result? The total mass entering the C90 grid cell will be the same as the net flux which would have entered the set of 4 cells at C180, and - as long as the distance calculation exactly matches that used internally by FV3 - that should result in the same amount of mass loss or accumulation in the cell. I think there might still be an error associated with differing calculations of the cell area/footprint, though. Is that correct or am I missing an important detail?

[tclune]

Yes. I could try to make it use "d1 + d2". Would not even be a lot of work. It just makes the error more subtle though, I think.

[sdeastham]

Wouldn't using d1 + d2 be worse? If my logic above is correct, using d12 directly in the regrid calculation as we already do should mean that errors due to precision problems don't actually cause a problem, even though it's mathematically incorrect?

[mathomp4]

Do the cool pictures change with this update? I like seeing them. 😄

[stale]

This issue has been automatically marked as stale because it has not had recent activity. If there are no updates within 7 days, it will be closed. You can add the "long term" tag to prevent the Stale bot from closing this issue.

[mathomp4]

Pinging @tclune. Should this stay open?

[tclune]

Nah - we know where to find it if @sdeastham returns to the issue.

[tclune]

@sdeastham Is this the bug you mentioned in the AIST meeting today?

[sdeastham]

It is! I couldn't recall what the conclusion was...

[mathomp4]

I've restored the bugfix/tclune/#199-hflux-fix-2nd-try branch. It is pretty out-of-date with develop and threw some conflicts when I tried to merge develop in. @tclune might need to do this because I'm not sure what to keep and what to not...

That said, if the conflicts can be fixed up. I'm glad to test out to see if it affects GEOSgcm in anyway.

[tclune]

Not sure how/why the Base_base stuff got mixed into this branch. Presumably we want all the other stuff. I'll try a hand merge.

[tclune]

OK- made an attempt. I had to do a new branch "3rd-try" because git would not let me push to origin after I rebased. Going to try to edit this PR, but may have to do a new one.