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

Re-factor of MOM_lateral_mixing_coeffs.F90 (VarMix) #539

Merged
merged 2 commits into from
Jul 5, 2017
Merged

Re-factor of MOM_lateral_mixing_coeffs.F90 (VarMix) #539

merged 2 commits into from
Jul 5, 2017

Conversation

adcroft
Copy link
Collaborator

@adcroft adcroft commented Jul 1, 2017

  • VarMix was originally for a crude implementation of a Visbeck et al.
    diffusivity formula. It has become a catchall for resolution functions,
    stored slopes and length-scales. It infact has never contained an actual
    diffusivity. The meaning of the parameter USE_VARIABLE_MIXING was original
    "use Visbeck" but has really meant "allocate and calculate everything in
    this module even if not being used". This has now been cleaned up...

  • USE_VARIABLE_MIXING still exists but does nothing.

    • It will be made obsolete shortly.
  • Visbeck related fields (VarMix@L2v, ...) are allocated only if
    KhTh_Slope_Cff>0 or KhTr_Slope_Cff>0.

  • The Eady growth rates are now allocated and calculated if
    either of Visbeck or MEKE schemes are in use.

  • The field VarMix%Rd_dx_h is allocated and calculated if either the
    diagnostic is requested or the field needed "Passivity" or the
    resolution functions.

  • The wave speed VarMix%cg1 is calculated only if needed but which
    includes resolution fns, Rd_dx, and FGNV stream fn.

  • Added logic in tracer_hordiff() to avoid referencing VarMix%L2v or
    VarMix%SN_v when VarMix%KhTh_Slope_Cff==0.

  • Added logic in tracer_hordiff() to avoid referencing VarMix%L2v or
    VarMix%SN_v when VarMix%KhTr_Slope_Cff==0.

  • This commit affects available_diags and MOM_parameter_doc for some
    experiments but does not changes answers.

- VarMix was originally for a crude implementation of a Visbeck et al.
  diffusivity formula. It has become a catchall for resolution functions,
  stored slopes and length-scales. It infact has never contained an actual
  diffusivity. The meaning of the parameter USE_VARIABLE_MIXING was original
  "use Visbeck" but has really meant "allocate and calculate everything in
  this module even if not being used". This has now been cleaned up...
- USE_VARIABLE_MIXING still exists but does nothing.
  - It will be made obsolete shortly.
- Visbeck related fields (VarMix@L2v, ...) are allocated only if
  KhTh_Slope_Cff>0 or KhTr_Slope_Cff>0.
- The Eady growth rates are now allocated and calculated if
  either of Visbeck or MEKE schemes are in use.
- The field VarMix%Rd_dx_h is allocated and calculated if either the
  diagnostic is requested or the field needed "Passivity" or the
  resolution functions.
- The wave speed VarMix%cg1 is calculated only if needed but which
  includes resolution fns, Rd_dx, and FGNV stream fn.
  - Closes #522.
- Added logic in tracer_hordiff() to avoid referencing VarMix%L2v or
  VarMix%SN_v when VarMix%KhTh_Slope_Cff==0.
- Added logic in tracer_hordiff() to avoid referencing VarMix%L2v or
  VarMix%SN_v when VarMix%KhTr_Slope_Cff==0.

- This commit affects available_diags and MOM_parameter_doc for some
  experiments but does not changes answers.
@adcroft adcroft requested a review from Hallberg-NOAA July 1, 2017 21:13
Copy link
Collaborator Author

@adcroft adcroft left a comment

Choose a reason for hiding this comment

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

@adcroft adcroft mentioned this pull request Jul 2, 2017
Copy link
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.

This refactoring is a valuable step toward cleaning up the code. I approve these changes, but I do wonder why we use a "create_group_pass" and then a "do_group pass" on lines 170-171 of MOM_lateral_mixing_coeffs.F90, instead of just doing the single line (and in this case more efficient) call to "pass_var".

@Hallberg-NOAA Hallberg-NOAA merged commit 7d401bc into mom-ocean:dev/gfdl Jul 5, 2017
@adcroft
Copy link
Collaborator Author

adcroft commented Jul 5, 2017

... but I do wonder why we use a "create_group_pass" and then a "do_group pass" ...

Agreed. I didn't add it, just carried it forward. There seems to have been a widespread replacement of single pass_var() with group passes. I prefer pass_var() when there's just the one field.

@adcroft adcroft deleted the cleanup-varmix branch July 13, 2017 01:13
nikizadehgfdl pushed a commit to nikizadehgfdl/MOM6 that referenced this pull request Oct 9, 2017
- MOM_parameter_doc files updated by NOAA-GFDL/MOM6@7d401bc and NOAA-GFDL/MOM6@13541f4
- No answer changes.

- NOAA-GFDL/MOM6@0192c3c Merge pull request mom-ocean#544 from adcroft/fix-hdfs-diagnostic
- NOAA-GFDL/MOM6@e9f146d Merge pull request mom-ocean#542 from adcroft/replace-keyword-variable-name
- NOAA-GFDL/MOM6@e05d8ae Merge pull request mom-ocean#541 from adcroft/fix-generic-tracer-MLD-arg
- NOAA-GFDL/MOM6@13541f4 Merge pull request mom-ocean#540 from adcroft/mle-length-scale
- NOAA-GFDL/MOM6@7d401bc Merge pull request mom-ocean#539 from adcroft/cleanup-varmix
- NOAA-GFDL/MOM6@b56e593 Added "skip_diags" argument to extractFluxes1d()
- NOAA-GFDL/MOM6@04116e8 Avoid allocating unused arrays in diabatic_driver
- NOAA-GFDL/MOM6@0f28767 Replaced array-syntax with loops
- NOAA-GFDL/MOM6@3eecc26 Replaced "sum" with "res" for variable name
- NOAA-GFDL/MOM6@6e4eaa4 Fixed argument name to match declaration gen_trcr_col_physics()
- NOAA-GFDL/MOM6@f733ff0 Implemented scale-aware option for MLE
- NOAA-GFDL/MOM6@a110df9 Fix VarMix cleanup with openMP
- NOAA-GFDL/MOM6@3e50273 Re-factor of MOM_lateral_mixing_coeffs.F90 (VarMix)
- NOAA-GFDL/MOM6@a1ac57f Merge pull request mom-ocean#538 from Hallberg-NOAA/dev/gfdl
- NOAA-GFDL/MOM6@fec16b9 Renamed variables "mod" to "mdl"
- NOAA-GFDL/MOM6@f6bade7 Merge branch 'dev/gfdl' of github.com:NOAA-GFDL/MOM6 into dev/gfdl
- NOAA-GFDL/MOM6@ebbce13 Merge branch 'dev/gfdl' of github.com:NOAA-GFDL/MOM6 into dev/gfdl
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