NCAR to main candidate branch (2022-06-03) #1571
Conversation
- Add Craik-Leibovich terms, vorticity and dynamic pressure components. - Add exploratory Stokes ddt version in MOM_wave_interface and MOM.F90 - Add tendency diagnostics for Stokes terms w/ control and option for diagnostic only Stokes force computation in MOM_CoriolisAdv.
- This only makes sense to add on the dynamics timestep.
Fix tidal mixing coeff diags when they are unavailable.
…o surfbands_refactor
Main to dev/ncar (after mom-ocean#1507 merge)
With these changes, arbitrary number of of partitioned stokes drift components may be imported from ww3 when coupled within cesm. After the planned unification of ww3 nuopc caps of CESM and EMC, the old (fixed) approach may be removed.
profile to restart file when surfbands wave coupling mode is on.
add the optional waves_csp arg to initialize_MOM calls so as to ensure that the waves_csp is allocated. This commit also has several misc doxygen fixes.
in MOM_CoriolisAdv. Also remove the Passive_Stokes_VF block since it's never set to true.
…ments incorrectly override dvdx and dudy when surfbands wave coupling is on.
- Adding logical to prevent using terms if not allocated - Adding optional diagnostic only output of term - Moving time increment to within dynamics step instead of in main step MOOM loop. - Correcting time tendency term to reflect time between Stokes drift updates and extent of dynamics time loop.
|
Regarding the answer changes observed by EMC, I am inviting @breichl to chime on why it may be the case. Brandon, this PR brings in the refactorings and bug fixes we have worked on several months ago. I believe EMC folks have the following wave set up, and they are seeing answer changes: |
yes we are using the above settings, see |
Address comment from reviewer by adding units to covTS and varS.
* Add ``implicit none ; private`` to this module; * Put module variables into the control structure for this module; * Add the description of the units for all real variables; * Add a consistent two-point indent throughout the module . TODO: Without further modifications, adding ``private`` to the control structure of this module will break the model. Currently, MOM.F90 needs access to ``use_stoch_eos``, ``stanley_coeff``, and some of the diagnostic ids.
I further tested UFS by turning wave off and got same answer as using main branch, so the issue is in the updated wave code |
|
@jiandewang I have set up an experiment that is similar to your configuration as close as possible, and pinpointed all of the bugfixes that cause the answer changes. Applying these changes in the main branch results in bitwise identical ocean.stats files when compared to the results of this candidate branch. (1) In mom_surface_forcing_nuopc.F90: (2) In MOM_wave_interface.F90: |
|
@alperaltuntas thanks, I will try that and get back to you. Just for confirmation: STOKES_VF, STOKES_PGF and STOKES_DDT need to be True in my test, right ? |
No, please set those to False as in your original configuration. |
|
@alperaltuntas I am bit confused here. I guess your git diff with main branch is based on your own branch which is not based on candidate branch |
|
I applied those diffs (bugfixes) to the main branch so as to make the main branch results the same as the results of this candidate. If you'd like to do the opposite, i.e., make the results of this candidate branch the same as the results of the main branch, then I guess you can revert those changes in the candidate branch instead. Essentially, what I did was to fix the bugs in main branch by applying those diffs. You can instead do the opposite and reintroduce those bugs to this main candidate to make the results the same. |
@alperaltuntas: still not clear here: |
|
@jiandewang, please apply the following change in this candidate branch and see if you can get bitwise identical results when compared to the main. Let me know if you still encounter answer changes. |
now I got the same answer as from main branch. Why ustkb and vstkb need to use pass_var instead of pass as vector ? I need document this in UFS as we will need to create a new baseline. |
|
The rationale is that both ustkb and vstkb are received from the wave component at T points, so during halo updates, they need to be treated as scalars and not vectors. |
now the final question is do we need to have a wavebug flag here ? It will be fine either way for UFS. |
|
@jiandewang If that's okay, my preference would be to not a have a wavebug flag, for the sake of keeping things more concise. |
@alperaltuntas sounds good, one of my team member is doing test in his system, will approve this PR once got confirmation. |
|
@alperaltuntas and @jiandewang, in the past we have opted not to introduce bug-flag in cases where the code was demonstrably not-self-consistent (e.g., changing layouts or changing between symmetric and non-symmetric memory leading to answer changes), because in that case it is not clear what previous solution needs to be recovered. In this case I could easily envision that there are internal self-inconsistencies with the previous code that fall into this category. |
In addition to bug fixes and general code improvements, this PR includes the following features:
I have verified that this PR does not change answers in MOM6-examples using intel/19.1.1.
New parameters
ENTHALPY_FROM_COUPLER(only via NUOPC cap)CFC_APPLY_NONLOCAL_TRANSPORTUSE_STANLEY_ISOSTANLEY_COEFFUSE_STANLEY_GMSTOCH_EOSUSE_STANLEY_ISOUSE_STANLEY_MLUSE_STANLEY_PGFObsolete parameters
STANLEY_PRM_DET_COEFFPGF_STANLEY_T2_DET_COEFFNew diagnostics
ENTHALPY_FROM_COUPLER=Trueheat_content_evaptotal_heat_content_evapCAu_StokesCAv_Stokesdvdt_Stokesdudt_Stokes3d_stokes_y_from_ddt3d_stokes_x_from_ddtPFv_StokesPFu_StokesObsolete diagnostics
heat_content_icemelttotal_heat_content_icemeltThe enthalpy associated with the mass from sea ice formation/melting is already accounted for in seaice_melt_heat. A note explaining this has been added to the code;
Features
Bug fixes
Testing, and cleanup
Contributors: