Candidate for dev/master 2017-08-17 #579
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
…ness, topography and ts.. stuffs.
- This commit adds the APIs (subroutines) that will be used for surface forcing in the Neverland configurations. - The forcing shapes are not yet coded.
…emplate Added a blank template Neverland_surface_forcing
- to avoid undesirable results near the top boundary.
…sirable behavior. - We made some changes to the wind stress for - y<=0.29 and 0.29<y<= 0.8-off, - and off value.
Some initial code changes to update the neutral_diffusion algorithm to find neutral surface positions using the discontinuous T and S reconstructions. Changes: - Break out a subroutine to return both the left and right edges of PPM reconstructions within a cell - Update logic to neutral_diffusion_calc_coeffs so that either the continuous or discontinuous reconstructions can be used ToDo: - Come up with a search algorithm that works on each subinterface - Update interpolate_for_nondim_position so that it returns a more accurate position of the neutral surface within each layer
search for the discontinuous positions can begin now.
…diff_update_neutral_position
…discontinuous reconstruction
of find_neutral_surface_positions_discontinuous routine. Will need to think about how this works and possibly rewrite the algorithm to loop over layers instead of by neutral surface index. ToDo: -Think about logic in the discontinuous reconstruction neutral surface algorithm, might need to be kl<=2*nk. Also what it means if neutral surface connects at a discontinuity Changes: -Some defined dimensions are now defined based on the total number of neutral surfaces, set at runtime to be 2*nk+2 if using a continuous reconstruction or 4*nk if using discontinuous
- Previously applyboundaryfluxesinout and extractbuoyancyflux2d both called extractfluxes1d. The second call was to get the surface buoyancy flux for ePBL and ignored calving/river contributions to the heat flux. This caused a diagnostic of the net heat flux to incorrectly also ignore these terms, corrupting this diagnostic. - This fix adds optional output of surface buoyancy flux to applyboundaryfluxesinout, therefore, no additional subroutine calls needed to get surface buoyancy flux for ePBL. - To avoid any change of answers this step required adding '_rate' outputs of netheat, netsalt, netmassinout, and pen_sw to extractfluxes1d. The net_heat_rate ignores calving and river contributions because the previous version ignored these terms. By adding the '_rate' outputs (w/ dt=1) we can preserve the old answers completely (avoiding a roundoff error), though this is less important of a point than ignoring the calving/river contributions to the heat flux. - A lengthy comment is added to diabatic_aux to detail the change and describe potential for simplifying (which would change answers). In principle all the '_rate' terms that are added here can be eliminated without compromising the mathematics, but we must confirm if we wish to include the calving/river contributions in the buoyancy flux used by ePBL. Otherwise, a second net_heat term is needed that is hard-coded to ignore calving/river contributions when computing the buoyancy flux used by ePBL.
find_neutral_surface_positions. Does not give expected results in tracer_mixing test case, more debugging needed.
still not as expected. Suspect a wrong value when setting position in either right or left search.
Corrected the scale conversion factor for pbce from H_to_m to m_to_H in a chksum call. Also corrected the units of pbce in several comments. This does change a set of diagnostic messages when DEBUG=True and H_TO_M is not 1, but all test cases are bitwise identical.
Added chksum calls for h_u and h_v if they are present as arguments to btcalc. All answers are bitwise identical.
GV%Angstrom was intended to be a very thin thickness in the same units (H) as the other thicknesses, but this conversion was not being done for Boussinesq test cases. Several test cases (e.g., double_gyre) that had previously changed answers when H_TO_M was not set to 1 now give the same answers when the value of H_TO_M is varied by a factor of integer powers of 2. Answers in test cases are unchanged.
Added Doxygen Comments to subroutines in parameterizations/vertical. All answers are bitwise identical
- Fixed conflicts in MOM_forcing_type.F90 and MOM_diabatic_driver.F90 - Cleaned up comments
Added a new optional argument, varname_prefix, to the variants of coupler_type_register_restarts that use a specified restart file, so that multiple coupler_type variables can use the same restart file. All existing answers are bitwise identical, but this new capability is required for the concurrent ice version of SIS2 to save fluxes by category and their weighted averages in the restart files, and it keeps these copies in synch with the intended versions for the next release of FMS.
Created a new subroutine, allocate_surface_state, to allocate the variables that MOM6 shares as its surface values, including the possibility of using an optional coupler_1d_bc_type argument to allocate the space associated with surface fluxes during initialization, which will then allow MOM6 to offer diagnostics associated with these fluxes. All answers are bitwise identical.
Changed an expression for a minimum value of ustar in set_diffusivity_init to always give a value with units of m/s, instead of H/s, reflecting the units used for ustar elsewhere. This will change answers if H_TO_M is not 1, but the existing reference solutions are unaffected.
Changed the exprsssion for the default value of TOLERENCE_ENT so that it does not vary with changes to H_TO_M. This could change answers (or at least parameter_doc files) if H_TO_M is not 1 (in which case the previous expression had mixed units), USE_REGRIDDING is false, and TOLERENCE_ENT is not set explicitly, but the existing reference solutions are unaffected.
Based on the suggestion of @Hallberg-NOAA, the array and scalar versions of the EOS functions were wrapped into interfaces. For each of the available equations of state, the array and scalar versions were also put behind interfaces. For the Wright EOS, the first and second deriviative scalar variants of the routines call the array variant by promoting the scalar T, S, and P to 1-element arrays and demoting the 1-element output arrays to scalars. Answers not expected to change.
Previous commit failed Travis test because of compiler-dependent evaluation of logical expressions. (if (kl>1 .and. bot_connected(kl-1)) could yield to an out of bounds error if the compiler evaluates bother sides of the statement. The offending line was moved one level below an explicit check for (kl>1)
574 diags reg no post
Changes made in #a49373c are correct in that the continuous polynomials should be evaluated to calculate gradients. However, they were intended to be implemented in a different set of commits. Answers should now be unchanged.
…hao/MOM6 into ashao-ndiff_update_neutral_position
…ff_referenced_density # Conflicts: # src/tracer/MOM_neutral_diffusion.F90
- Makes pipeline robust to changes in FMS, SIS2, etc. by doing clean
builds.
- Also makes system robust to multiple PRs being tested in parallel.
- Used a cache directory containing tar files with artifacts
- gitlab artifacts are transferred to the server and these artifacts
are far too big for that.
- Separates layout test from symmetric - non-symmetric comparison.
- Adds gnu restart test.
- Adds gnu static test.
Note: I elected to use gnu rather than intel for the restart/static
tests because lustre file errors seem to be most often associated with
the intel executable - I've no idea why that would be.
- Not all artifacts were cleaned up due to an extra character in the cleanup stage.
Passed last week: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/pipelines/2258 Also passed today on the newly refactored pipeline: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/pipelines/2294
Passed tests: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/pipelines/2295 Updates MOM_parameter_doc files so MOM6-examples needs to be updated.
|
Looks good! Does not change answers for my small OBC cases compared to my branch. |
|
This branch has run-time errors (out of bounds) in neutral_diffusion @ashao Reverting MOM_neutral_diffusion.F90 to 6e73103 works fine. I think this line should not have a +1 in the declaration of ref_pres: |
- Fixes declaration of ref_pres in MOM_neutral_diffusion.F90 to avoid run-time OOB error reported by NCAR (@gustavo-marques). - NCAR are using the same version of Intel (16.0.3) that GFDL uses for testing but evidently have better compiler options that caught this - none of our tests failed. - No answer changes.
|
Good catch @gustavo-marques . I've just patched this branch with your fix (da8f186). It still passes all our tests (https://gitlab.gfdl.noaa.gov/ogrp/MOM6/pipelines/2322). It is curious that we are using the same version of the Intel compiler but we didn't catch this. You might have better compiler options so we should review ours. |
|
Works fine for me now!
…On Tue, Aug 22, 2017 at 10:33 AM, Alistair Adcroft ***@***.*** > wrote:
Good catch @gustavo-marques <https://github.com/gustavo-marques> . I've
just patched this branch with your fix (da8f186
<NOAA-GFDL@da8f186>).
It still passes all our tests (https://gitlab.gfdl.noaa.gov/
ogrp/MOM6/pipelines/2322).
It is curious that we are using the same version of the Intel compiler but
we didn't catch this. You might have better compiler options so we should
review ours.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<NOAA-GFDL#579 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AK0FgblNAEHRVnskHMujskYa0XeUpWAtks5sawLZgaJpZM4O67pf>
.
--
Gustavo M. Marques
National Center for Atmospheric Research phone: 303-497-174
<(303)%20497-1737>*0*
Climate & Global Dynamics fax : 303-497-1700
<(303)%20497-1700>
PO Box 3000
Boulder, CO 80307-3000
http://www.gustavo-marques.com
|
|
Just for completeness, it looks like the OOB issue reported above was also caught by the NCI tests in build 125: https://accessdev.nci.org.au/jenkins/job/mom-ocean.org/job/MOM6_runtime_analyzer/125/ |
|
Thanks for catching/fixing that, Gustavo. Also, I just tried it on my
laptop (without the fix) with GNU v6.3.0 and it also triggered the OOB.
…On Tue, Aug 22, 2017 at 5:25 PM, Alistair Adcroft ***@***.***> wrote:
Just for completeness, it looks like the OOB issue reported above was also
caught by the NCI tests in build 125: https://accessdev.nci.org.au/
jenkins/job/mom-ocean.org/job/MOM6_runtime_analyzer/125/
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<NOAA-GFDL#579 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ADquB_8LgEZaW6oOu1tTqiLsvXTcfdxGks5sa0dSgaJpZM4O67pf>
.
|
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 will bring dev/master up to date with dev/gfdl as of 2017-08-17.
As for #550 and #578, we will wait for the NCAR and ESMG forks to evaluate the PR before we merge.
For this PR:
Enjoy the eclipse! 🌞