+Refactored and replaced ALE_main #213
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## dev/gfdl #213 +/- ##
============================================
- Coverage 37.11% 36.98% -0.13%
============================================
Files 263 262 -1
Lines 73062 72727 -335
Branches 13644 13586 -58
============================================
- Hits 27114 26899 -215
+ Misses 40929 40836 -93
+ Partials 5019 4992 -27
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Why does dzRegrid change values? |
|
dzRegrid does not actually change values. The spuriously failing TC test occurs because the post_data call for dZregrid has been moved before the post_data call for other routines. If we had chosen to do a sort on the chksum_diag files before doing the diff between the version generated by the previous version of the code and the new one, they would have been identical. |
Started the refactoring of code related to ALE_main, including: - Split remap_all_state_vars into remap_tracers and remap_velocities - Created the new public subroutine pre_ALE_diagnostics, and call it from step_MOM - Eliminate the unused argument conv_adjust to regridding_main - Eliminated check_grid, and moved these tests into regridding_main - Eliminate check_remapping_grid, and replace it with calls directly to check_grid_column inside of remapping_main All answers are bitwise identical, but there are new public interfaces and changes to the arguments of other public interfaces.
Continued the refactoring of code related to ALE_main, including: - Added the new public subroutine pre_ALE_adjustments - Added the new arguments h_new, dzRegrid, and PCM_cell to ALE_main - Added the new arguments h_new and dzRegrid to ALE_offline_main - Moved the code copying the new thickness to the primary thickness out of ALE_main and into step_MOM_thermo All answers are bitwise identical, but there are new public interfaces and changes to the arguments of other public interfaces.
Move the post_data call for dzRegrid to immediately follow its calculation in regridding main. All answers and diagnostics are bitwise identical, but because this commit changes the order in which some diagnostics are posted, the TC testing falsely reports a failure due to changed diagnostics.
Replaced ALE_main with ALE_regrid and calls to ALE_remap_tracers and ALE_remap_velocities from step_MOM_thermo. Also eliminated ALE_main_offline and ALE_offline_tracer_final, as they can now be replaced with a calls to ALE_regrid and a call to ALE_remap_tracers. Also added unit descriptions to a number of comments describing variables in MOM_ALE.F90. All answers are bitwise identical, but there are 3 new publicly visible subroutines (ALE_regrid, ALE_remap_tracers, and ALE_remap_velocities) and three previous interfaces (ALE_main, ALE_main_offline and ALE_offline_tracer_final) have been eliminated.
Hallberg-NOAA
force-pushed
the
refactor_ALE_main
branch
from
9c7c34e to
3be0907
Compare
adcroft
approved these changes
Oct 20, 2022
There was a problem hiding this comment.
This is a good step in the re-structuring of the ALE methods.
Testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/17145
adcroft
pushed a commit
to adcroft/MOM6
that referenced
this pull request
Dec 7, 2022
* 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 <[email protected]> * 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) (NOAA-GFDL#9) * Changed Stanley stochastic coefficient from exp(X) to exp(aX) * Extra spaces removed * Stoch eos init fix (NOAA-GFDL#10) * Don't bother calculating tv%varT if stanley_coeff<0 * Missing then added * Merge Ian Grooms Tvar Discretization (NOAA-GFDL#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 <[email protected]> * Multiplied tvar%SGS by grid cell thickness ratio * Added limiter for tv%varT * Stoch eos ncar linear disc (NOAA-GFDL#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 <[email protected]> * 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-GFDL#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-GFDL#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-GFDL#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 <[email protected]> * 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-GFDL#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 <[email protected]> Co-authored-by: Philip Pegion <[email protected]> Co-authored-by: Jessica Kenigson <[email protected]> Co-authored-by: Jessica Kenigson <[email protected]> Co-authored-by: jkenigson <[email protected]> Co-authored-by: jskenigson <[email protected]> Co-authored-by: Jessica Kenigson <[email protected]> Co-authored-by: Jessica Kenigson <[email protected]> Co-authored-by: Philip Pegion <[email protected]> Co-authored-by: Jessica Kenigson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR includes a series of commits that refactor the capabilities that had
been wrapped up in ALE_main, giving greater flexibility in the determination of
the variables that are remapped, and allowing for a reduction in the number of
different routines within MOM_ALE.F90. This PR takes essential preliminary
steps toward addressing #203. The specific changes include:
check_grid_column inside of remapping_main
ALE_main and into step_MOM_thermo
other diagnostics
ALE_remap_velocities from step_MOM_thermo
with calls to ALE_regrid and ALE_remap_tracers
All answers are bitwise identical (including in cases testing the offline
tracer capability), but there are new public interfaces, and the fact that the
order with which some diagnostics are posted leads to a false report of changes
to the diagnostics with the automated TC testing.
The commits in this PR include: