Remove discontinuities in the balance op by guillaumevernieres · Pull Request #885 · JCSDA-internal/soca

guillaumevernieres · 2023-05-11T23:52:15Z

Description

This is not going to suddenly massively improve our forecast skills or analysis, but the filtering of the jacobians of the linearized balances was not handled properly.
What this PR does:

replaces the box filter to a tanh filter when zeroing out jacobians over part of the water column
the handling of the min/max limits for dS(T)/dT was wrong (set to 0 instead of the limits). This issue is illustrated here, thanks to @hyunsookkim-NOAA for the plot.
limiting the application of dS/dT to below the mixed layer is also done with the same tanh filter
a slight reorganization of the yaml config for the balance operator, it looks something like this now:

       - linear variable change name: BalanceSOCA
        kst:
          dsdtmax: 0.1
          dsdzmin: 3.0e-6
          dtdzmin: 1.0e-6
          nlayers: 999
        ksshts:
          nlayers: 10
        dcdt:
          filename: data_static/72x35x25/dcdt.nc
          name: dcdt

where nlayers is the index at which the filter = 0.5 ... No parameter to configure the slope.

A "bit" out of scope for this PR, but I also added a vanilla option for the vertical convolution. It's meant to by-pass a chunk of configuration that was meant to confuse people with slight dyslexia ...

Issue(s) addressed

Testing

How were these changes tested?

ctest/5deg
1/4 deg global
some earlier versions were tested by @hyunsookkim-NOAA
What compilers / HPCs was it tested with?
intel
gnu
Are the changes covered by ctests? (If not, tests should be added first.)
yes

Dependencies

A change of yaml config will be necessary in a few downstream repos (soca-sci, gdasapp, ...)

travissluka · 2023-05-12T16:45:14Z

src/soca/LinearVariableChange/Balance/soca_balance_mod.F90

-           if (layer_depth%val(i,j,k) < mld%val(i,j,1)) then
-              jac(k) = 0.0_kind_Real
-           end if
+           coef_mld = soca_tanh_filt(layer_depth%val(i,j,k),mld%val(i,j,1))


I know you don't totally agree with how the nemovar balances were implemented, but I believe they set the jacobian to 0 at the surface and linearly interpolate down to the MLD, instead of such a sharp cutoff. Thoughts on trying that (someday?)

travissluka · 2023-05-12T16:51:35Z

src/soca/LinearVariableChange/VertConv/soca_vertconv_mod.F90

  lz = max(lz, self%scale_layer_thick*abs(hocn%val(i,j,:)))

+  ! if doing vanilla vertconv, we're done!
+  if (self%vanilla) return


whats the point of adding this flag? It's the same effect as setting Lz_mld: 0 in the yaml

🤦‍♂️
🍨 ?

travissluka · 2023-05-12T17:06:41Z

test/testinput/3dvar_godas.yml

+          dsdtmax: 0.1
+          dsdzmin: 3.0e-6
+          dtdzmin: 1.0e-6
+          nlayers: 999


@guillaumevernieres I see several different values of nlayers depending on the tests here. What values would you recommend for the real-world config?

@travissluka We need to turn it off for the global until we properly test it and add a high latitude limit. 999 would turn it off. Omitting the kst config in the yaml to turnt kst off might be a better idea.
10 maybe in the hat10?

…ated

travissluka

Thanks for doing this. 2 minor things, but otherwise looks good. (also, would it be worthwhile to add the capability, someday, to output the jacobians so that we can look at them to help with debugging?)

remove the vanilla stuff and use the existing paramters
instead of cramming in a 999, make it so that you can omit kst or ksshts in the yaml to turn off those sections?

guillaumevernieres · 2023-05-12T20:20:17Z

Thanks for doing this. 2 minor things, but otherwise looks good. (also, would it be worthwhile to add the capability, someday, to output the jacobians so that we can look at them to help with debugging?)

remove the vanilla stuff and use the existing paramters

instead of cramming in a 999, make it so that you can omit kst or ksshts in the yaml to turn off those sections?

Will do. Thanks for the review @travissluka .

travissluka

Thanks, @guillaumevernieres
Since soca-science CI is currently broken, I'll go ahead and merge this and try to get the associate soca-sci changes merged in later today.

* Use SABER blocks chaining for training (#852) * Adapt to name changes in ufo (#867) * Use VertInterp instead of MarineVertInterp (#874) * remove linear var change from ensevariance ctest (#881) * Bugfixes for BUMP (#869) * A few fixes to the Model to Analysis variable change (#847) * Remove discontinuities in the balance op (#885) * set to min/max instead of 0 * ... * vanilla vert conv * tidy sign * removed jac_mask * tanh filt * updated yamls and test refs * merge develop and update answers after the develop history MESS i created * code tidy/removed vanilla --------- Co-authored-by: Travis Sluka <tsluka@ucar.edu> * default plots (#888) Co-authored-by: Travis Sluka <tsluka@ucar.edu> * add VADER to SOCA (#887) * add vader to build, but dont use it * add State::fromFieldSet * variablechange "uses" vader * switched to ens. templ. (#894) * Implementation of EnsMeanAndVariance (#892) * added the ensmeanandvariance app * ... --------- Co-authored-by: Travis Sluka <tsluka@ucar.edu> * Read the std. dev. instead of the variance (#896) * skip sqrt, saving optional * forgot test yaml * Update test/testinput/varchange_bkgerrsoca_stddev.yml Co-authored-by: Travis Sluka <tsluka@ucar.edu> --------- Co-authored-by: Travis Sluka <tsluka@ucar.edu> * Use the generic oops interpolator (#898) * use oops interpolation * to_fieldset_ad * forgot some halo masks * fix horizfilt * fix some test answers * letkf workaround for missing val bug * work around for getvalues test * change obs input to have 1 guaranteed land point * Update version (#899) * remove ewok repo (#889) Co-authored-by: Yannick Trémolet <30638944+ytremolet@users.noreply.github.com> * Merge B matrix-related applications (#890) * Merge B matrix-related applications * Trigger tests --------- Co-authored-by: Benjamin Menetrier <benjamin.menetrier@irit.fr> Co-authored-by: Travis Sluka <tsluka@ucar.edu> * update for VADER PR#586 (#902) * add ModelData * fix coding norms * Update CMakeLists.txt * Fix vertical gradient (#914) * a fix dvdz * Updating test references --------- Co-authored-by: Hyun-Chul.Lee <Hyun-Chul.Lee@noaa.gov> * libs in soca * Commend out dirac.x test and disable ecbuild_install_project as we did for fv3-jedi prevent the following error: -- Configuring done CMake Error in soca/src/soca/CMakeLists.txt: Target "soca" INTERFACE_LINK_DIRECTORIES property contains path: "/Users/heinzell/work/ufs-bundle/20230714/ufs-bundle-use-jedi-develop/soca/src/soca/." which is prefixed in the source directory. -- Generating done CMake Generate step failed. Build files cannot be regenerated correctly. * Apply same bug fix as for fv3-jedi to fix missing linker path for MPI libraries due to ESMF bug --------- Co-authored-by: Benjamin Menetrier <benjamin.menetrier@irit.fr> Co-authored-by: Wojciech Śmigaj <w.smigaj@gmail.com> Co-authored-by: Travis Sluka <tsluka@ucar.edu> Co-authored-by: Guillaume Vernieres <guillaume.vernieres@noaa.gov> Co-authored-by: Yannick Trémolet <30638944+ytremolet@users.noreply.github.com> Co-authored-by: Benjamin Menetrier <30638301+benjaminmenetrier@users.noreply.github.com> Co-authored-by: Kriti Bhargava <kritib@ucar.edu> Co-authored-by: Hyun-Chul.Lee <Hyun-Chul.Lee@noaa.gov> Co-authored-by: Dom Heinzeller <dom.heinzeller@icloud.com>

guillaumevernieres requested review from kbhargava, liuxiao37k and travissluka May 12, 2023 12:59

travissluka reviewed May 12, 2023

View reviewed changes

travissluka force-pushed the develop branch from 947737f to bdfad33 Compare May 12, 2023 19:32

guillaumevernieres and others added 8 commits May 12, 2023 13:48

set to min/max instead of 0

8484154

...

dd0c5b9

vanilla vert conv

2fce3cf

tidy sign

a77d360

removed jac_mask

6ceb6d2

tanh filt

2ae9fda

updated yamls and test refs

0bfd935

merge develop and update answers after the develop history MESS i cre…

79b18b7

…ated

travissluka force-pushed the bugfix/balance branch from fd52b31 to 79b18b7 Compare May 12, 2023 20:03

travissluka requested changes May 12, 2023

View reviewed changes

code tidy/removed vanilla

7ccd65d

guillaumevernieres requested a review from travissluka May 18, 2023 17:19

travissluka approved these changes May 19, 2023

View reviewed changes

travissluka merged commit 2d7be80 into develop May 19, 2023

travissluka deleted the bugfix/balance branch May 19, 2023 15:28

guillaumevernieres mentioned this pull request May 19, 2023

Update soca bkgerr yaml NOAA-EMC/GDASApp#480

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove discontinuities in the balance op #885

Remove discontinuities in the balance op #885
travissluka merged 9 commits intodevelopfrom
bugfix/balance

guillaumevernieres commented May 11, 2023

Uh oh!

travissluka May 12, 2023

Uh oh!

guillaumevernieres May 12, 2023

Uh oh!

travissluka May 12, 2023

Uh oh!

guillaumevernieres May 12, 2023

Uh oh!

travissluka May 12, 2023

Uh oh!

guillaumevernieres May 12, 2023

Uh oh!

travissluka left a comment

Uh oh!

guillaumevernieres commented May 12, 2023

Uh oh!

travissluka left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

guillaumevernieres commented May 11, 2023

Description

Issue(s) addressed

Testing

Dependencies

Uh oh!

travissluka May 12, 2023

Choose a reason for hiding this comment

Uh oh!

guillaumevernieres May 12, 2023

Choose a reason for hiding this comment

Uh oh!

travissluka May 12, 2023

Choose a reason for hiding this comment

Uh oh!

guillaumevernieres May 12, 2023

Choose a reason for hiding this comment

Uh oh!

travissluka May 12, 2023

Choose a reason for hiding this comment

Uh oh!

guillaumevernieres May 12, 2023

Choose a reason for hiding this comment

Uh oh!

travissluka left a comment

Choose a reason for hiding this comment

Uh oh!

guillaumevernieres commented May 12, 2023

Uh oh!

travissluka left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants