Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

+(*)Rescale opacity and avoid segmentation faults #43

Merged
merged 6 commits into from
Dec 29, 2021

Conversation

Hallberg-NOAA
Copy link
Member

This PR rescales the internal units of opacity for dimensional consistency
testing, and it includes changes that avoid segmentation faults if PEN_SW_NBANDS=0.
It also includes two other commits that were previously included in the
pending dev/gfdl PR #37, but which overlapped with the opacity code changes and
prevented the model from encountering other segmentation faults.

There are unit changes in an intent(out) array argument to a public interface,
extract_optics_slice(), and there are changes in the units of one element in the
optics_type. The optics_type probably should have been opaque (to follow more
object-oriented thinking about code-style), but it is not being treated this way
by the MOM_generic_tracer module to avoid an unnecessary copy of a 4-d array.

There is also a change in the type of an argument to to the publicly visible
function optics_nbands(), from an optics_type to a pointer to an optics type, to
better reflect how it was actually being used, and to avoid segmentation faults
if the runtime parameter PEN_SW_NBANDS = 0. Two other opacity routines now
handle the case with PEN_SW_NBANDS = 0 by simply returning early, before they
attempt to make use of the contents of the optics_type that was passed to them.

All answers and output are bitwise identical in any cases that worked before.
The commits in this PR include:

  • 0544f9f2e +(*)Avoid segmentation faults if PEN_SW_NBANDS = 0
  • 08cd63b85 Merge branch 'dev/gfdl' into opacity_rescale
  • 049241ce1 +Rescaled optics%opacity_band
  • 5172c495c Warn if opacity_from_chl is called without fluxes
  • d7337145e (*)Fix extract_diabatic_member

  Return the diabatic_aux_CSp from extract_diabatic_member it is present as an
optional argument.  Somehow this was omitted when this routine was created, but
without this correction the offline tracer mode returns a segmentation fault.
Also, added the proper conversion factor in the register_diag_field call for
e_predia, and internally calculate the interface heights in units of [Z ~> m]
for dimensional consistency testing.  All answers are bitwise identical in
cases that ran before.
  Issue a warning with a helpful message if opacity_from_chl is called with no
shortwave fluxes, and added logical tests to avoid a segmentation fault later in
this routine.  This should not happen, as it makes no sense, but it was
occurring with the offline tracer code, and can be avoided by setting
PEN_SW_NBANDS=0 if there are no shortwave fluxes to penetrate.  Also turned the
real dimensional parameter op_diag_len into a variable and set it immediately
before where it is used.  Many spelling errors were also corrected in
MOM_opacity.F90.  All answers are identical in cases that ran before.
  Rescaled the units of optics%opacity_band from [m-1] to [Z-1 ~> m-1], with the
opacity values returned from extract_optics_slice also rescaled by the same
factor, which can be offset by compensating changes to the opacity_scale
argument.  Also rescaled 4 other internal variables and documented the units on
3 more.  One uncommon parameter (SW_1ST_EXP_RATIO) listed the wrong units in its
get_param call, and this was corrected, but turned out not to have been logged
for any of the MOM6-examples test cases.  Some compensating changes were also
made in the MOM_generic_tracer module, which directly accesses the contents of
the optics_type (thereby preventing it from being opaque).  All answers and
output in the MOM6-examples test suite are bitwise identical.
  Modified three routines in MOM_opacity to avoid segmentation faults if
PEN_SW_NBANDS = 0, and hence if the optics type is not being allocated.  In the
case of optics_nbands(), this involved formally changing an optics_type argument
into a pointer to an optics_type.  (Pointers to an optics_type were already been
used as the argument in all calls to optics_nbands(), but it was not always
associated.)  In two other routines, the change is simply to add a return call
if there are 0 bands of shortwave radiation.  With these changes, the single
column test cases with no penetrating shortwave radiation now successfully run
if PEN_SW_NBANDS = 0 and give answers that are identical to those obtained with
PEN_SW_NBANDS = 1.  All answers and output in cases that ran previously are
bitwise identical, but there is a subtle change in a public interface.
@codecov
Copy link

codecov bot commented Dec 16, 2021

Codecov Report

Merging #43 (a8a2039) into dev/gfdl (d9d82e3) will decrease coverage by 0.00%.
The diff coverage is 29.09%.

Impacted file tree graph

@@             Coverage Diff              @@
##           dev/gfdl      #43      +/-   ##
============================================
- Coverage     28.92%   28.91%   -0.01%     
============================================
  Files           242      242              
  Lines         71233    71252      +19     
============================================
+ Hits          20601    20605       +4     
- Misses        50632    50647      +15     
Impacted Files Coverage Δ
src/tracer/MOM_generic_tracer.F90 0.00% <0.00%> (ø)
src/parameterizations/vertical/MOM_opacity.F90 29.12% <27.50%> (-0.23%) ⬇️
...parameterizations/vertical/MOM_diabatic_driver.F90 38.80% <50.00%> (-0.03%) ⬇️
...arameterizations/vertical/MOM_bulk_mixed_layer.F90 32.07% <100.00%> (ø)
...rc/parameterizations/vertical/MOM_diabatic_aux.F90 34.67% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9d82e3...a8a2039. Read the comment docs.

@marshallward
Copy link
Member

@Hallberg-NOAA can you look into the merge conflicts?

@marshallward
Copy link
Member

Gaea regression: https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/14449 ✔️

@marshallward marshallward merged commit 1028ee0 into NOAA-GFDL:dev/gfdl Dec 29, 2021
@Hallberg-NOAA Hallberg-NOAA deleted the opacity_rescale branch January 3, 2022 13:33
@Hallberg-NOAA Hallberg-NOAA added bug Something isn't working documentation Improvements or additions to documentation refactor Code cleanup with no changes in functionality or results labels Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation refactor Code cleanup with no changes in functionality or results
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants