Add COAPS/FSU IAU capability #68
Conversation
Update to EMC fork
This is a basic framework to use NCODA increments on layer space for ODA incremental updates.
P_inc has been replaced by h of analysis state, add options to use full fields instead of increments and add diagnostics to get outputs of increments (for now to check what your increment looks like if full fields).
Increments calculation moved at initialization, updates applied over nstep instead of nstep+1 and increments diagnostics has been added
…rection and general clean up - Add option to use or not uv increment - The calculation of the increment when using full field has been moved at initialization - A SSH correction has been applied to h_obs to account for the different SSH between the background and target state - The incremental update is now done over nstep-1 - a general clean up has been done with variable name change to make the code easier to follow
jiandewang
requested review from
jiandewang,
DeniseWorthen and
JessicaMeixner-NOAA
|
From Bob Hallberg:
I would be happy to comment again after this has been revised. |
|
Alex made a bunch of changes based on Bob’s review. I have not looked at them yet, and I still need to add in a IAU regression test
…Sent from my iPhone
On Jun 3, 2021, at 9:44 PM, jiandewang ***@***.***> wrote:
From Bob Hallberg:
There are a few things with this prototype of a commit that pop out at first look.
First, I think that the fields that are being updated in apply_oda_incupd() should be passed in as arguments to help make it more explicit what is going on. This could be (h, u, v, tv) or (h, u, v, T, S), but the way that there are pointers being stored to these arrays, and then their contents are being changed under the covers makes it less explicit what is going on. In the other places where these arrays are used but not change, it might be less problematic, but if we can conveniently rewrite those with explicit arguments replacing the pointers to external fields, that would be preferable too.
There are a few places that use complicated (but probably correct) array syntax expressions like hv_obs(1:nz_data) = (sum(hv(1:nz))/sum(hv_obs(1:nz_data))) *hv_obs(1:nz_data) that appear to be correct but could introduce machine or compiler dependencies. We generally try to avoid using complicated array syntax expressions. (See the discussion in https://github.com/NOAA-GFDL/MOM6/wiki/Code-style-guide .)
It looks like the incremental updates are cumulative, so they will probably need to appear in the restarts.
There are some places where some of the ..._answers_2018 parameters have been reproduced in MOM_oda_incupd.F90 from other code. These flags are just there to reproduce old answers, and for any new code the right option is just to code them as though ..._answers_2018 = .false. in all cases.
I would be happy to comment again after this has been revised.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
…ility - remove the u,v,tv pointers from the set_up_oda_incupd subroutines - explicitly put tv,u,v in arguments of apply_oda_incupd - change name get_oda_increments to calc_oda_increments - explicitly put tv,u,v in arguments of calc_oda_increments - use loops to calculate sum of h’s - remove the 2018_answers options - added option to output increments when using full fields - added subroutine output_oda_incupd_inc to write a separate double-precision file for the increments. - add if (mask==1 ) condition to apply/calculate the increments -add pass_var,pass_vector on increments and u,v,tv at the end of calc_oda_increments and apply_oda_incupd respectively - added CS%ncount to restart file to have reproducibility on restart. - Added option ODA_INCUPD_RESET_NCOUNT (! default= True, If True, reinitialize number of updates already done.)
|
I have tested the updates on hera. I have also added a regression test for MOM6-IAU. |
|
@pjpegion : I am going to ask Bob to check the updated code. |
|
comments from Bob Hallberg (June 25): After going through this PR visually, it seems to me like it is reasonable. I would also like to run it past Matt Harrison, though, to make sure that he doesn't see anything where this might either conflict with or excessively duplicate the other ODA code that we have (although I do not expect this). There are a few minor things that I would have done differently to follow the typical MOM6 syntax as documented in the style guide (https://github.com/NOAA-GFDL/MOM6/wiki/Code-style-guide), like avoiding array-syntax math, documenting the units of all real variables, using a longer and more descriptive variable name than just 'q', correct the indent of the end subroutine statement or adding spacing around assignments that I would suggest. However, but these are all very minor things that can be corrected later, and I think that the overall structure of these additions might be about right. There are a few more self-consistency tests that I would like to suggest that you try before putting in the PR. First, please verify that you get identical answers if you use a different PE count and layout - I am expecting this to succeed, but we should test it. Second, please verify that you get identical answers if you do the dimensional consistency tests by using values of [TZLRQ]_RESCALE_POWER in your MOM_input files that are something other than 0. In doing these tests, it is advisable that each of the T, Z, L, R, and Q should be scaled by different values. We should get the bitwis identical solutions with any combination of values, but if the various units are all scaled by the same power, bugs can still slip through. Please try your test with something like T_RESCALE_POWER = 13, Z_RESCALE_POWER = -7, L_RESCALE_POWER = 23, R_RESCALE_POWER = 4, Q_RESCALE_POWER = 29. From my inspection of the code in this PR, I would not be surprised if we need to add rescaling of some of the input fields, but the tests are the right way to figure this out. |
|
from Mat Harrison (June 28): These updates seem like they will be mergeable . You are correct to route the updates through the diabatic driver, which differs from the approach taken in the SPEAR configuration . I suspect that we may want to similarly route the updates in the online ensemble filter. In SPEAR, Feiyu is applying a background climatological increment via file, in addition to the online updates, so there will be some initial duplication of capabilities . Having two ODA control structures (ODA_incup_CS and ODA_CS) is also potentially confusing, but we can work on streamlining the two. Feiyu and I will be making a PR shortly for the online code used in SPEAR. I'm not sure if having online ensemble filter capabilities is within the scope of your applications, but we should be able to interface with UFS code within the algorithm (e.g. for observational data streaming , covariance operators, ...) if needed. |
|
I clean up the code to follow MOM6 coding standard, and added a rescale factor for ODA_INCUPD_NHOURS. I get zero diffs when running on different process layouts and when testing with RESCALE_POWER |
|
I will work with Phil to push back to GFDL once IAU PR and GFDL"s 20210708 main branch updating are in EMC repo. |
|
UFS RT finished on all machines except wcoss which is down at this moment, per discussion with Jun we decide to skip the wcoss RT. |
Alex Bozec and Alan Wallcraft from COAPS/Florida State University have developed Incremental analysis update capability for MOM6. This has been tested in the UFS-datm with a "replay" to ORAS5 analysis.