[Feature] `delnflux` all nord #3

FlorianDeconinck · 2024-02-08T21:16:09Z

The original port of DelnFlux used the 3 first values of the nord_dictionary to compute. The original code reads the nord value for all K-levels and decides to execute the code or not.

GEOS use case showed that the assumption in pyFV3 is incorrect. On top of that, the time "optimized" by the strategy is minimal and by virtue of the DSL should not lie in the hands of the modelers.

In this PR:

We add version.py to introduce a IS_GEOS switch that can be used to differentiate between numerical choices between NOAA and NASA
Full nord coverage for DelnFlux
Add check on column calculation taken as hypothesis for downstream calculations
FV3GFS/GEOS differences solving:
- d2_high_order_stencil as a difference of orders operations
- nord & damp column calculation goes all the way to the n_sponge layer in GEOS
NOTE: to do FV3GFS/GEOS swap we use the constant file in ndsl, a known refactorable entity.

GEOS/FV3GFS: d2_highord_stencil ops order difference

Add extra check on the value of nord which is taken as an hypothesis downstream

Vorticity damping GEOS differs from FV3GFS

oelbert

One question for Lucas, one question for Florian, and one change to d_sw

pyFV3/stencils/d_sw.py

pyFV3/version.py

pyFV3/stencils/delnflux.py

oelbert · 2024-02-12T16:26:45Z

pyFV3/stencils/delnflux.py

+):
+    with computation(PARALLEL), interval(...):
+        if nord > current_nord:
+            d2 = ((fx - fx[1, 0, 0]) + (fy - fy[0, 1, 0])) * rarea


@lharris4 Mathematically this is the same as the version used in SHiELD, but with parens around the fx/fy subtraction to enforce the order of operations. Is it ok to merge these together or should we keep separate versions for GEOS and NOAA applications?

I think this is OK although the former may make it easier for the compiler to optimize. Is there any particular reason to do it this way?

At double floating precision, this has minimal to no impact. At single precision, however the order of the subtraction has measurable impacts (especially because of close-to-zero behavior of a lot of those codes).

A large amount of those diff pattern have been corrected in the GEOS fork of FV3.

Great. We'd like to include a number of these in the GFDL FV3 also.

oelbert

This looks good. It would be nice to be able to combine the d2_higherorder stencils if possible, but that can happen in a subsequent PR. I'll open an issue for it.

lharris4 · 2024-02-21T15:00:53Z

Hi, Florian. Thank you for letting me know. We hadn't seen these changes in the GEOS fork of FV3 before; I know GMAO had been working on some order-of-operation changes to avoid rounding-error mass loss at single precision but hadn't heard whether they were successful. This would potentially be useful for the GFDL main branch too. Lucas

…

On Wed, Feb 21, 2024 at 9:47 AM Florian Deconinck ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In pyFV3/stencils/delnflux.py <#3 (comment)>: > ***@***.*** -def d2_highorder(fx: FloatField, fy: FloatField, rarea: FloatField): - d2 = (fx - fx[1, 0, 0] + fy - fy[0, 1, 0]) * rarea - return d2 +def d2_highorder_stencil_GEOS( + fx: FloatField, + fy: FloatField, + rarea: FloatFieldIJ, + nord: FloatFieldK, + d2: FloatField, + current_nord: int, +): + with computation(PARALLEL), interval(...): + if nord > current_nord: + d2 = ((fx - fx[1, 0, 0]) + (fy - fy[0, 1, 0])) * rarea At double floating precision, this has minimal to no impact. At single precision, however the order of the subtraction has measurable impacts (especially because of close-to-zero behavior of a lot of those codes). A large amount of those diff pattern have been corrected in the GEOS fork of FV3. — Reply to this email directly, view it on GitHub <#3 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMUQRVCDHVYMVT2PM3STD4LYUYCGJAVCNFSM6AAAAABDAOETFKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQOJTGQYTENRTGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

FlorianDeconinck · 2024-02-21T15:13:04Z

Hi, Florian. Thank you for letting me know. We hadn't seen these changes in the GEOS fork of FV3 before; I know GMAO had been working on some order-of-operation changes to avoid rounding-error mass loss at single precision but hadn't heard whether they were successful. This would potentially be useful for the GFDL main branch too. Lucas

My pleasure. If this project can have the side effect of updating people on work that hasn't been diffused then I am doubly glad! I'll chat with Bill about it before we merge this.

FlorianDeconinck · 2024-02-27T14:29:38Z

@oelbert / @lharris4

As expected, those changes were part of the work to keep mass conservation at single precision. GEOS is running single precision by default for a while with good results on that side.

Going forward I'll push those changes in as fixes rather than code differences between the two agencies.

FlorianDeconinck · 2024-02-27T14:37:26Z

@oelbert Ready for merge (once tests come back happy)

oelbert

I'm happy with this 👍

fmalatino

Approved

lharris4 · 2024-02-27T18:06:19Z

pyFV3/stencils/delnflux.py

+):
+    with computation(PARALLEL), interval(...):
+        if nord > current_nord:
+            d2 = ((fx - fx[1, 0, 0]) + (fy - fy[0, 1, 0])) * rarea


Great. We'd like to include a number of these in the GFDL FV3 also.

lharris4 · 2024-02-27T18:07:36Z

pyFV3/stencils/delnflux.py

@@ -1240,38 +639,40 @@ def __call__(self, q, fx2, fy2, damp_c, d2, mass=None):
        """

        if mass is None:
-            self._d2_damp(q, d2, damp_c)
+            self._d2_damp(q=q, d2=d2, damp=damp_c, nord=self._nord)


Note that some of these damping coefficients are layer-dependent (ie. much stronger and lower-order damping in the sponge layer).

FlorianDeconinck added 2 commits February 8, 2024 16:05

Computation is done looking at the n_ord on the I,J,K grid point

330168c

GEOS/FV3GFS: d2_highord_stencil ops order difference

FV3GFS/GEOS: nord/damp calculation to n_sponge differs

45f4d14

Add extra check on the value of nord which is taken as an hypothesis downstream

FlorianDeconinck requested a review from oelbert February 8, 2024 21:16

FlorianDeconinck added 6 commits February 8, 2024 16:16

lint

1122949

bad include

596010b

lint

8c898f2

Introduce version.py to define IS_GEOS switch based on constant

a98a0c6

lint

4c8b427

typo

0f6445a

FlorianDeconinck changed the title ~~Feature/delnflux all nord~~ [Feature] delnflux all nord Feb 9, 2024

Use configuration to init "nord_v"

a2e0ca6

Vorticity damping GEOS differs from FV3GFS

oelbert requested changes Feb 12, 2024

View reviewed changes

FlorianDeconinck added 2 commits February 12, 2024 12:56

Make sure nord_w and nord_t are correct

cf6aef3

Fix all nord init

cadd976

FlorianDeconinck requested a review from oelbert February 12, 2024 20:36

FlorianDeconinck changed the base branch from main to develop February 16, 2024 16:33

oelbert previously approved these changes Feb 20, 2024

View reviewed changes

oelbert requested a review from fmalatino February 20, 2024 20:38

Swap d2_highorder_stencil for f32 mass conservation fixed one

ffc0e8c

FlorianDeconinck dismissed oelbert’s stale review via ffc0e8c February 27, 2024 14:36

FlorianDeconinck requested a review from oelbert February 27, 2024 14:36

lint

2065a98

FlorianDeconinck added 3 commits February 27, 2024 09:43

Adapting error threshold for mass fix changes

738a7eb

Adapting error threshold for mass fix changes

e97edec

Bad yml formatting

b012254

oelbert approved these changes Feb 27, 2024

View reviewed changes

fmalatino requested a review from lharris4 February 27, 2024 17:04

fmalatino approved these changes Feb 27, 2024

View reviewed changes

lharris4 approved these changes Feb 27, 2024

View reviewed changes

fmalatino merged commit 1780efb into develop Feb 27, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] `delnflux` all nord #3

[Feature] `delnflux` all nord #3

FlorianDeconinck commented Feb 8, 2024 •

edited

Loading

oelbert left a comment

oelbert Feb 12, 2024

lharris4 Feb 20, 2024

FlorianDeconinck Feb 21, 2024

lharris4 Feb 27, 2024

oelbert left a comment

lharris4 commented Feb 21, 2024 via email

FlorianDeconinck commented Feb 21, 2024 •

edited

Loading

FlorianDeconinck commented Feb 27, 2024

FlorianDeconinck commented Feb 27, 2024

oelbert left a comment

fmalatino left a comment

lharris4 Feb 27, 2024

lharris4 Feb 27, 2024

[Feature] delnflux all nord #3

[Feature] delnflux all nord #3

Conversation

FlorianDeconinck commented Feb 8, 2024 • edited Loading

oelbert left a comment

Choose a reason for hiding this comment

oelbert Feb 12, 2024

Choose a reason for hiding this comment

lharris4 Feb 20, 2024

Choose a reason for hiding this comment

FlorianDeconinck Feb 21, 2024

Choose a reason for hiding this comment

lharris4 Feb 27, 2024

Choose a reason for hiding this comment

oelbert left a comment

Choose a reason for hiding this comment

lharris4 commented Feb 21, 2024 via email

FlorianDeconinck commented Feb 21, 2024 • edited Loading

FlorianDeconinck commented Feb 27, 2024

FlorianDeconinck commented Feb 27, 2024

oelbert left a comment

Choose a reason for hiding this comment

fmalatino left a comment

Choose a reason for hiding this comment

lharris4 Feb 27, 2024

Choose a reason for hiding this comment

lharris4 Feb 27, 2024

Choose a reason for hiding this comment

[Feature] `delnflux` all nord #3

[Feature] `delnflux` all nord #3

FlorianDeconinck commented Feb 8, 2024 •

edited

Loading

FlorianDeconinck commented Feb 21, 2024 •

edited

Loading