Skip to content

User/aja/diag interp and extrinsic#1

Merged
nichannah merged 17 commits into
nichannah:dev/masterfrom
adcroft:user/aja/diag_interp_and_extrinsic
Sep 20, 2016
Merged

User/aja/diag interp and extrinsic#1
nichannah merged 17 commits into
nichannah:dev/masterfrom
adcroft:user/aja/diag_interp_and_extrinsic

Conversation

@adcroft
Copy link
Copy Markdown

@adcroft adcroft commented Sep 7, 2016

FYI, this is the code I added to handle interfaces and extrinsic variables. I'm holding back on pushing to dev/master until you and I agree figure out a path forward - although I would like to use the new interface and extrinsic diagnostics in CM4 soon. I'm making the PR against your dev/master so you can see the changes - comparing against your rho-sigma-diagnostic-regridding branch is less revealing.

The main conflict is your removal of the axes pointer (was called remap_axes) inside the diag_type. I added lots of logical inside the axes_grp type to efficiently decide whether to remap or interpolate and then added one more logical to the diag_type to override the remapping behavior for extrinsic data (already vertically integrated). I also added other logicals inside axes_grp (is_u_point) in order to avoid the complex is_u_axes() function we were using. I think you were solving the same problems by exposing the underlying 1D axes ids?

Nicholas Hannah and others added 9 commits September 6, 2016 04:19
- The umo,vmo CMOR diagnostics were using in-place global multiplications
  which is known to lead to unitialized variable errors within the diag
  manager.
- Registering with conversion=GV%H_to_kg_m2 instead so that the
  diag_mediator can create a local array with which to do the conversion.
- No answer changes.
- register_diag_field now only has a two-way split and (used to be four
  for native, CMOR, z, z-CMOR). Call tree is now:
  - register_diag_field() calls register_diag_field_expand_cmor() for both
    the native and z-remapped field.
  - register_diag_field_expand_cmor() calls register_diag_field_expand_axes() for both
    the native names and CMOR names. This is where the diag_type is allocated and filled.
  - register_diag_field_expand_axes() calls register_diag_field_fms() one of four
    ways depending on presence of area interp_method optional arguments.
- Fix: The z-diagnostic cell_methods was incorrectly using the native vertical
  dimension name for some fields when the diagnostic was not requested. This
  led to the wrong names being provided in available_diags. The re-factor corrected
  this unexpectedly.
- No answer changes.
- Removed functions is_u_axes(), is_v_axes(), is_B_axes(), is_layer()
  by storing logicals in the axes_group type.
- Also added needs_remapping logical for non-native 3D axes groups
  to indicate to do remapping.
- No answer changes.
- Interface variables cannot be remapped so our z-coordinate diagnostics
  have been limited to layer-centered fields. This implements interpolation
  of interface-centered fields to the interfaces of the target grid (same as
  used by the diagnostic remapping).
- This added a new group of axes for the z-coordinate interfaces.
- Note that the (horizontal) B-grid located diagnostics are not implemented
  (as they are not implemented in remapping code either).
- Had to provide a work-around for the current existence of MOM_diag_to_Z
  which calls register_diag_field for 3D fields that should not be remapped.
  Todo: defaults for is_native should be flipped.
- No answer changes.
- Some layer data is already vertically integrated (e.g. hT, uh, vh, ...)
  and cannot be remapped as if it were a concentration (intrinsic variable).
  Setting optional argument v_extrinsic=.true. to register_diag_field() now
  indicates that the field should be re-gridded as an extrinsic field.
- No answer changes.
- 3D diagnostics h, uh, vh have been labeled as vertically extrinsic
  (meaning integrated over the layer) so that remapping does not
  treat the fields as concentrations.
- Changes availabl_diags.
- No answer changes.
@adcroft
Copy link
Copy Markdown
Author

adcroft commented Sep 7, 2016

I should add that I renamed "remap_axes" within diag_type to "axes" because even native diagnostics use the pointer now but the native axes have axes%is_native=.true.. Previously, we determined whether to remap based on whether the remap_axes pointer was associated.

- New subroutines and functions should be doxygenized at inception.
- The vector diag_remap was a vector of control structures. To help
  me understand the new code I'm renaming by adding _cs.
- Note this code does not compile due to a broken merge of
  nicjhan-334-rho-sigma-diagnostic-regridding on dev/master.
- For each diag_remap_ctrl element there are now also eight axes_groups
  just as there are eight native axes_groups.
- Not being used yet!
- Still doesn't compile after a broken merge of
  nicjhan-334-rho-sigma-diagnostic-regridding on dev/master.
- The coexistence of axes_groups and axes ids is confusing. I've renamed
  %axes to $axes_ids to help me understand the code.
- Note this code does not compile due to a broken merge of
  nicjhan-334-rho-sigma-diagnostic-regridding on dev/master.
- This keeps the new name "ZSTAR" but also allows use of the old
  coordinate name "Z*".
- Note this code does not compile due to a broken merge of
  nicjhan-334-rho-sigma-diagnostic-regridding on dev/master.
…o-sigma-diagnostic-regridding

- Moved new inteprolation and reintegration routines from MOM_diag_mediator
  (added on branch user/aja/diag_interp_and_extrinsic) to MOM_diag_remap.
- Added vectors of axes_groups to correspond to each diag_remap_cs.
- Added workarounds for keeping _z suffix in module name and input
  parameter names with ZSTAR.
- Compiles and passes regression tests.
- A few quick plots indicate remapping, interpolation and reintegration
  are working as well as they were on branch user/aja/diag_interp_and_extrinsic.

# Conflicts:
#	src/framework/MOM_diag_mediator.F90
- When the source or destination columns were completely vanished the
  reintegrate_column() in MOM_diag_vkernels was getting stuck in a
  never ending loop.
- Added exit tests for these special cases.
- Also added unit tests to cover these scenarios.
- To avoid a conflict with the general coordinate diagnostics, I've
  renamed the diagnostics module registered within MOM_diag_to_z to
  ocean_model_zold.
@adcroft
Copy link
Copy Markdown
Author

adcroft commented Sep 9, 2016

This now has all of your branch (PR NOAA-GFDL#348) and my branch merged together. Everything compiles, runs, and passes tests for me but can you review and test it? Once you're satisfied, if you merge this into your branch I'll then merge onto dev/master using the original PR (which will automatically update with your branch).

Open issues (for later refinement):

  • In register_diag_field() I still have an explicit registration for the native variable. I believe in your branch you had done away with this and were handling the native registration using the 'LAYER' coordinate. If that's preferred we could do the same here by adding to L1005-L1024 and set the pointer to the argument 'axes'. Currently, 'LAYER' is a no-op so long as DIAG_REMAP_LAYER_GRID_DEF is unset but if it were I think we would get FATALs due to double registering to ocean_model.
  • The code that interprets the coordinate parameters is mostly duplicated between MOM_diag_remap.F90 and MOM_ALE.F90 (it always was). It would be better (thinking ahead to adding more coordinates) to re-use the code in MOM_ALE somehow.
  • All the new code I wrote is doxygenized [hint].
  • For testing I've left the list of diagnostics coordinates as you had it (i.e. all coordinates) but in practice I think we want to limit to a select few. New diagnostic coordinates that we will provide soon for CMIP6 will be 'RHO0' and 'RHO2' which are the same as 'RHO' but with the reference pressure set to 0db and 2000db respectively. As we grow the list of coordinates I would like to avoid the list of input parameters growing unintentionally too.
    • Alternative to limiting the list, we could add a run-time parameter "NUM_DIAG_COOD" and use numbers in the coordinate parameters, DIAG_COORD_1="RHO2", DIAG_REMAP_GRID_DEF_1="FILE:..."?

@nichannah nichannah merged commit 73207b5 into nichannah:dev/master Sep 20, 2016
@nichannah
Copy link
Copy Markdown
Owner

OK, that's a huge step forward with this feature. I'm a lot happier with how the axes are being handled (thanks!) and I think we're converging on a nice way to do generalised diagnostic vertical remapping/interpolation.

I've just added a single commit to clean up comments and remove some unused code / type members. I then pushed this to NOAA-GFDL#348 . It's possible that github is confused because I merged into master first ... lots of new / unexpected commits have turned up in NOAA-GFDL#348 . I'll check this.

I've created an issue to improve the way vertical axis configuration is being done: NOAA-GFDL#355

nichannah pushed a commit that referenced this pull request Oct 26, 2016
Proposed update to ashao:origin/offline tracers. I agree that renaming the flag from DO_ONLINE to OFFLINE_TRACER_MODE is more descriptive. I checked that this still works as intended with the Baltic_ALE_z_offline_tracers and updated the MOM_override for that test case in NOAA-GFDL/MOM6-examples#110
@adcroft adcroft deleted the user/aja/diag_interp_and_extrinsic branch April 25, 2017 14:26
nichannah pushed a commit that referenced this pull request Oct 29, 2018
removed copy_profiles, using linked profiles in ensemble_filter
nichannah pushed a commit that referenced this pull request Oct 30, 2018
Pull latest MOM6 changes into dev/gfdl fork
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