Skip to content

Memory management cleanup and initialization fixes#1413

Merged
Hallberg-NOAA merged 3 commits into
mom-ocean:dev/gfdlfrom
marshallward:valgrind_fixes
Jun 8, 2021
Merged

Memory management cleanup and initialization fixes#1413
Hallberg-NOAA merged 3 commits into
mom-ocean:dev/gfdlfrom
marshallward:valgrind_fixes

Conversation

@marshallward
Copy link
Copy Markdown
Collaborator

This patch fixes up and enables several of the derived type destructor
functions (_end()) used to handle memory management of the model.

  • Two uninitialized logical flags which would cause errors in optimized
    builds have also been fixed.

    • Visbeck_S_max in thickness diffusion is now initialized to zero,
      which acts to disable it in subsequent operations.

    • remap_answers_2018 is now unset when NDIFF_CONTINUOUS is false.

  • Several of the destructor functions were restructured so that they do
    not explicitly deallocate their pointer inputs. This will allow us to
    stage the functions as formal destructors for the derived types via
    the final keyword in a later patch. It also allows us to pass the
    inputs on stack rather than as pointers.

    • barotropic_end
    • continuity_end
    • CoriolisAdv_end
    • deallocate_MOM_domain
    • diabatic_driver_end
    • geothermal_end
    • hor_visc_end
    • MEKE_end
    • MOM_CVMix_conv_end
    • MOM_CVMix_ddiff_end
    • MOM_CVMix_shear_end
    • MOM_diagnostics_end
    • MOM_regridding_end
    • MOM_sum_output_end
    • PressureForce_end
    • set_diffusivity_end
    • thickness_diffuse_end
    • tidal_forcing_end
    • VarMix_end
    • vertvisc_end
  • In a few cases, the deallocations were re-ordered to match the
    reversed order of allocation.

    • MOM_CVMix_conv_mod
    • MOM_CVMix_ddiff_mod
    • MOM_CVMix_shear_mod
  • A few constructors always initialized their control structures, even
    when disabled. In some of these cases, the allocation is now skipped
    if the corresponding feature is disabled.

  • diag_mediator_end now includes the following changes:

    • axes_grp_end was introduced to deallocate axes_grp types.
      The remap_axes* are now deallocated.

    • We now cycles through the diagnostic list and deallocate the
      supplemental diagnostics.

    • Downsampled diagnostic masks and remaps are now deallocated

  • The initialized blockName of the param file parser are now
    dellocated before reinitializing them. Although the existing value is
    still lost, it is at least now deallocated from heap.

  • A bug was fixed in hor_visc_end; Smag_Ah and Leith_Ah areas were
    incorrectly swapped.

The principal motivation for this work is to eliminate any errors
detected by valgrind, and to integrated automated memcheck testing to
the verification test suite.

marshallward and others added 3 commits May 27, 2021 23:23
This patch fixes up and enables several of the derived type destructor
functions (`_end()`) used to handle memory management of the model.

- Two uninitialized logical flags which would cause errors in optimized
  builds have also been fixed.

  * `Visbeck_S_max` in thickness diffusion is now initialized to zero,
    which acts to disable it in subsequent operations.

  * `remap_answers_2018` is now unset when `NDIFF_CONTINUOUS` is false.

- Several of the destructor functions were restructured so that they do
  not explicitly deallocate their pointer inputs.  This will allow us to
  stage the functions as formal destructors for the derived types via
  the `final` keyword in a later patch.  It also allows us to pass the
  inputs on stack rather than as pointers.

  * `barotropic_end`
  * `continuity_end`
  * `CoriolisAdv_end`
  * `deallocate_MOM_domain`
  * `diabatic_driver_end`
  * `geothermal_end`
  * `hor_visc_end`
  * `MEKE_end`
  * `MOM_CVMix_conv_end`
  * `MOM_CVMix_ddiff_end`
  * `MOM_CVMix_shear_end`
  * `MOM_diagnostics_end`
  * `MOM_regridding_end`
  * `MOM_sum_output_end`
  * `PressureForce_end`
  * `set_diffusivity_end`
  * `thickness_diffuse_end`
  * `tidal_forcing_end`
  * `VarMix_end`
  * `vertvisc_end`

- In a few cases, the deallocations were re-ordered to match the
  reversed order of allocation.

  * `MOM_CVMix_conv_mod`
  * `MOM_CVMix_ddiff_mod`
  * `MOM_CVMix_shear_mod`

- A few constructors always initialized their control structures, even
  when disabled.  In some of these cases, the allocation is now skipped
  if the corresponding feature is disabled.

- `diag_mediator_end` now includes the following changes:

  * `axes_grp_end` was introduced to deallocate axes_grp types.
    The `remap_axes*` are now deallocated.

  * We now cycles through the diagnostic list and deallocate the
    supplemental diagnostics.

  * Downsampled diagnostic masks and remaps are now deallocated

- The initialized `blockName` of the param file parser are now
  dellocated before reinitializing them.  Although the existing value is
  still lost, it is at least now deallocated from heap.

- A bug was fixed in `hor_visc_end`; Smag_Ah and Leith_Ah areas were
  incorrectly swapped.

The principal motivation for this work is to eliminate any errors
detected by valgrind, and to integrated automated memcheck testing to
the verification test suite.
Copy link
Copy Markdown
Collaborator

@Hallberg-NOAA Hallberg-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have examined all of these changes, and I agree with the direction that they are heading, although it should be noted that there is still more to do along these lines.

This PR conditionally passed the pipeline testing at https://gitlab.gfdl.noaa.gov/ogrp/MOM6/-/pipelines/12831, but because the code after this PR no longer logs the parameter VISBECK_MAX_SLOPE in all cases, there will have to be a manual update to the MOM_parameter_doc files in MOM6-examples as soon as this PR is merged into dev/gfdl.

@Hallberg-NOAA Hallberg-NOAA merged commit da287e1 into mom-ocean:dev/gfdl Jun 8, 2021
@marshallward marshallward deleted the valgrind_fixes branch October 20, 2021 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants