Update to GFDL 20200401 dev/master #18
Conversation
First stab at parameterizing the diabatic mixing by mesoscale eddies using a 'bulk layer' approach. Added a simple unit test where column thickness is exactly equal to the boundary layer depth, equal, layer thicknesses, and the tracer gradient points from right to left. Go Gustavo and Andrew
Add more complex unit tests and begin work on improving the algorithm to deal with cases where the boundary layer intersects within a layer.
For the near-boundary lateral mixing, the indices of the layers that are spanned by the boundary layer need to be returned. Additionally, in cases where the boundary layer intersects partway through a layer, the non-dimensional position also needs to be returned for polynomial reconstructions to be evaluated correctly. Six unit tests were added to test this new function. All unit tests currently pass
Many updates to allow the boundary layer to intersect a layer. Commented out some of the unit test previously added as the API has changed. These need to be revisited later.
…hen computing fluxes
- Add two unit tests for cases where the surface boundary layer intersects partly through a cell. 1. Right column same BLT, same thicknesses, flux from right to left, constant in the vertical 2. Right column same BLT, same thicknesses, flux from right to left, linear profile on right TODO: 1. Uncomment out previous unit tests 2. Update API in those test cases 3. Need to add similar unit tests for the bottom boundary
This updates all the previously commented out unit tests to update the API. These changes were required to allow for cases where the boundary layer that intersects partway through a model layer.
All the development in the boundary layer mixing scheme has focused on simple unit tests. This provides a skeleton for some of the interfaces that will need to be in place before using the new parameterization in a 'real' MOM6 simulation
- Pass diabatic CS through tracer_hor_diff_init and lateral_boundary_mixing_init. - Modify extract_diabatic_member to return KPP and ePBL CS - Finish initialization for lateral_boundary_mixing
The new lateral boundary mixing routine has been added into tracer_hor_diff and needs to be tested in a 'real' configuration. This only works with KPP for now because ePBL needs US passed which is not currently implemented in the API for tracer_hor_diff
Calculation of fluxes needs to be masked otherwise NaNs will definitely be calcualted
The CVMix KPP module would allocate it's control structure regardless of wthether KPP was used or not. The allocate statement has been moved down after USE_KPP has been parsed.
- Indexing error in the y-direction led to a non-conservation of tracer - Extra guards added to avoid divisions by zero - Pass US through to lateral_boundary_mixing to enable compatibility with ePBL
Diffusive fluxes calculated from the lateral boundary mixing scheme of tracers have been added as a diagnostic to the tracer registry. The total 'bulk' flux was added as well
The get_MLD and get_BLD routines only return boundary layer depths on the T-grid's computational domain leading to striping when calculating the LBM fluxes. Adding a halo update for this variable fixes the problem
TODO: * add code for boundary = BOTTOM * add unit tests
|
@jiandewang : can you explain what change you are making in this commit (jiandewang@b41878b) |
|
I did a diff between the branch in this PR and GFDL's dev/master and the only difference was: which we added to run in debug mode. |
|
Jessica: you are right, that piece of code was added by Denise in order to run in debug mode |
|
Denise: this time GFDL dev/master merged NCAR-dev-master-candidate-2020-03-27 branch, as always, there is no detail explanation on what's new |
|
@jiandewang There's some detail of what was committed in the GFDL PR here https://github.com/NOAA-GFDL/MOM6/pull/1081 I think @DeniseWorthen was asking to know more details about how the conflict was resolved in the commit you made here: jiandewang@b41878b |
conflict is at "mom_surface_forcing_nuopc.F90" on the use of "kg_m2_s_conversion"
|
|
there is no input change. |
|
I don't understand what you are saying is a conflict. If I look at GFDL's repo for master (010492e) when they merged in ncar's pr, the thing that changed is the name of the fields. Those should be what we have already since ncar had merged our field name changes to dev/ncar back when we were working on this. When you merge in GFDL's master candidate, are you pushing the merge prior to fixing all the conflicts? |
|
conflict appeared during merge stage so have to solve it before push. line 447 in dev/emc |
|
@jiandewang I think I figured out where the confusion is coming from. When you resolved the conflicts from the merge, added the file and then went to commit, you put the commit message "solve conflice in mom_surface_forcing_nuopc.F90 " whereas usually with a merge commit we're used to seeing something like "Merge remote-tracking branch..." |
|
@JessicaMeixner-NOAA is right. And I personally at least think it leads to a lot of confusion in the commit history. I had struggled earlier trying to unwind the commit history at the Jan 2020 commit probably for the same reason. I think it makes much more sense to have the commit message be 'merge remote ...' Otherwise the commit message implies to me that you merged in gfdl and then made a second commit where you changed something in dev/emc. |
|
fully agree, should contain more info in commit history. |
|
@jiandewang could we fix this on this PR and going forward? This would mean re-doing what you've done; the other option might be to amend your commit message that you've already pushed. Googling says that is possible but seems a bit complicated (?) The commit message would be 'merge remote ' etc and then additionally on the next line you could add that you fixed conflict in the subroutine. |
|
one simple solution is adding one line in README.md, commit with note of what we need, let me know if this sounds good or not. |
|
That would mean we would constantly have to fix conflicts and maintain a file that is purposefully different from dev/master. I think that adds more work for the future. When we push back to GFDL, NCAR or others they would always have to go find that we added things to the README that they do not want, etc. I think the commit message history is the right place for this information. |
|
@JessicaMeixner-NOAA Yes, I agree. We need a clean commit history that clearly indicates that we've merged to master. I personally don't think it is necessary to have noted what conflicts were fixed, but if want to include that additional information then a second line in the commit message is the place to do it. |
jiandewang
force-pushed
the
update-to-GFDL-20200327
branch
from
b41878b
to
e8c05f1
Compare
|
what about now: |
|
Can you do a git --amend and change it to read: merge GFDL dev/master 20200401 commit (hash # 010492e) |
solve conflice in mom_surface_forcing_nuopc.F90
jiandewang
force-pushed
the
update-to-GFDL-20200327
branch
from
e8c05f1
to
6aec544
Compare
|
done. |
Merge in latest dev/gfdl updates
Bug fix for reading First_direction from restart
* initial hooks for stochastic EOS modifications * remove debug statements * add documentation * Change ampltiude from 0.39 to sqrt(.39) * remove global_indexing logic from stoch_eos_init * switch to using MOM_random and add restart capability * update random sequence to update each each time-step * remove tseed0 from MOM_random (leftover from debugging) * Added necessary submodules and S^2, T^2 diagnostics to MOM_diagnostics * Added diagnostics for outputting variables related to the stochastic parameterization. * Diagnostics in MOM_PressureForce_FV updated for stochastic (rather than deterministic) Stanley SGS T variance parameterization. * Added parentheses for reproducibility. * Changed diagnostics to account for possible absence of stoch_eos_pattern in MOM_PressureForce_FV, when deterministic parameterization is on. * remove mom6_da_hooks and geoKdTree from pkg * add stochastic compoment to MOM_thickness_diffuse * fix array size declaration and post_data * Corrected indexing of loops in MOM_calc_varT * Changed how parameterization of SGS T variance (deterministic and stochastic) is switched on in PGF and thickness diffusion codes * Corrected a few typos * Cleaned up indices, redundant diagnostic, printing * Fixed diagnostic IDs * Fixed diagnostics typo * Corrected indices in calculation of tv%varT * Minor index fix * Corrected bug in pressure in Stanley diagnostics * Fixed whitespace error * Stoch eos clock (#5) *Added a clock for the Stanley parameterization Co-authored-by: jkenigson <jkenigso@gmail.com> * add halo update to random pattern * Update MOM_stoch_eos.F90 Fix bug for looping over compute domain (is -> isc etc.) * Avoid unnessary computations on halo (MOM_stoch_eos) and code clean-up (MOM_thickness_diffuse) * Removed halo updates before determ param calc * Update MOM_stoch_eos.F90 Removed unnecessary code * Bug - indices are transposed * Changed Stanley stochastic coefficient from exp(X) to exp(aX) (#9) * Changed Stanley stochastic coefficient from exp(X) to exp(aX) * Extra spaces removed * Stoch eos init fix (#10) * Don't bother calculating tv%varT if stanley_coeff<0 * Missing then added * Merge Ian Grooms Tvar Discretization (#11) * Update MOM_stoch_eos.F90 In progress updating stencil for$ | dx \times \nabla T|^2$ calculation * New discretization of |dx\circ\nablaT|^2 Co-authored-by: Ian Grooms <ian.grooms@colorado.edu> * Multiplied tvar%SGS by grid cell thickness ratio * Added limiter for tv%varT * Stoch eos ncar linear disc (#12) * Update MOM_stoch_eos.F90 In progress updating stencil for$ | dx \times \nabla T|^2$ calculation * New discretization of |dx\circ\nablaT|^2 * AR1 timescale land mask Adds land mask to the computation of the AR1 decorrelation time * Update dt in call to MOM_stoch_eos_run The call to `MOM_stoch_eos_run` (which time steps the noise) is from within `step_MOM_dynamics`. `step_MOM_dynamics` advances on time step `dt` (per line 957), but the noise is updated using `dt_thermo`. It seems more appropriate to update the noise using `dt`, since it gets called from within `step_MOM_dynamics`. * Fixed the units for r_sm_H * Remove vestigial declarations The variables `hl`, `Tl`, `mn_T`, `mn_T2`, and `r_sm_H` are no longer used, so I removed their declarations and an OMP private clause Co-authored-by: Ian Grooms <ian.grooms@colorado.edu> * Update MOM_thickness_diffuse.F90 Changed index for soft convention * Update CVMix-src * Ensure use_varT, etc., initialized * Don't register stanley diagnostics if scheme is off * Stanley density second derivs at h pts (NOAA-EMC#15) * Change discretization of Stanley correction (drho_dT_dT at h points) * Limit Stanley noise, shrink limiting value * Revert t variance discretization * Reverted variable declarations * Stanley scheme in mixed_layer_restrat, vert_fill in stoch_eos, code cleanup (NOAA-EMC#19) * Test Stanley EOS param in mixed_layer_restrat * Fix size of TS cov, S var in Stanley calculate_density calls * Test move stanley scheme initialization * Added missing openMP directives * Revert Stanley tvar discretization (NOAA-EMC#18) * Perform vertical filling in calculation of T variance * Variable declaration syntax error, remove scaling from get_param * Fix call to vert_fill_TS * Code cleanup, whitespace cleanup Co-authored-by: Jessica Kenigson <jessicak@cheyenne1.cheyenne.ucar.edu> * Use Stanley (2020) variance; scheme off at coast * Comment clean-up * Remove factor of 0.5 in Tvar * Don't calculate Stanley diagnostics on halo * Change start indices in stanley_density_1d * Stanley param in MOM_isopycnal_slopes (NOAA-EMC#22) Stanley param in MOM_isopycnal_slopes and thickness diffuse index fix * Set eady flag to true if use_stored_slopes is true * Cleanup, docs, whitespace * Docs and whitespace * Docs and whitespace * Docs and whitespace * Whitespace cleanup * Whitespace cleanup * Clean up whitespace * Docs cleanup * use_stanley * Update MOM_lateral_mixing_coeffs.F90 * Adds link to another TEOS10 module * Set Stanley off for testing * Line continuation Co-authored-by: Phil Pegion <38869668+pjpegion@users.noreply.github.com> Co-authored-by: Philip Pegion <Philip.Pegion@noaa.gov> Co-authored-by: Jessica Kenigson <jessicak@cheyenne6.cheyenne.ucar.edu> Co-authored-by: Jessica Kenigson <jessicak@cheyenne3.cheyenne.ucar.edu> Co-authored-by: jkenigson <jkenigso@gmail.com> Co-authored-by: jskenigson <jessica.kenigson@colorado.edu> Co-authored-by: Jessica Kenigson <jessicak@cheyenne1.cheyenne.ucar.edu> Co-authored-by: Jessica Kenigson <jessicak@cheyenne5.cheyenne.ucar.edu> Co-authored-by: Philip Pegion <ppegion@Philips-MacBook-Pro.local> Co-authored-by: Jessica Kenigson <jessicak@cheyenne4.cheyenne.ucar.edu>
GFDL updated dev/master on 20200401, need to update EMC repository (issue #17 )
It has been tested in ufs-s2s-model successfully without answer change.
run log can be found on HERA at /scratch1/NCEPDEV/stmp2/Jiande.Wang/scrub/rtgen.3186/tmp/log