Correct pkg/mnc grid parameter naming

[IvanaEscobar]

What changes does this PR introduce?

Update pkg/mnc grid names to match MDS

What is the current behaviour?

Grid variable names are unique when using pkg/mnc, leading to increased complexity when comparing to MDS output in post processing. eg. 'HFacC' instead of 'hFacC'

What is the new behaviour

Grid variable names match MDS

Does this PR introduce a breaking change?

no

Other information:

left mnc_cw_write_grid_info.F in pkg/mnc as is since it has a clear warning that it is unsupported and unused.

Suggested addition to tag-index

update pkg/mnc grid names to match mds
provide backwards compatibility with MATLab and Python tools

[mjlosch]

@IvanaEscobar I like this suggestion. It removes many unnecessary differences between MDS and MNC output. HFacC to hFacC makes total sense. With dxG to DXY and similar there's the complication that in the xmitgcm output, the variables are called dxG, rAz, etc. (and XG, inconsistently). Maybe that's something to discuss?

Probably, more people use xmitgcm (which is actually quite dormant currently), than the MNC output option, but I can't be sure.

[jm-c]

I have few comments (going along @mjlosch remarks):

1. The current names in NetCDF grid files are not the best choice (the "HFacC" being one example), so it's probably a good idea to have some remaining.
2. We need to agree on new names. I like the idea of matching some existing names (here binary file names) but could also decide on something else (e.g., how the variable is written in the MITgcm code ?).
3. We need to think of backward compatibility issue. Would this be left to the post-processing utils (matlab/python/julia) or have an option in the code to keep the old name ? I tend to prefer the former option but, in this case, it would be good to have an example how to do this, e.g., in utils/matlab/load_grid.m

[IvanaEscobar]

I have few comments (going along @mjlosch remarks):

1. The current names in NetCDF grid files are not the best choice (the "HFacC" being one example), so it's probably a good idea to have some remaining.
2. We need to agree on new names. I like the idea of matching some existing names (here binary file names) but could also decide on something else (e.g., how the variable is written in the MITgcm code ?).
3. We need to think of backward compatibility issue. Would this be left to the post-processing utils (matlab/python/julia) or have an option in the code to keep the old name ? I tend to prefer the former option but, in this case, it would be good to have an example how to do this, e.g., in utils/matlab/load_grid.m

To address concerns, there were no backwards compatibility issues when creating changes within pkg/mnc grid variable names. This is because modifications here affect only post-processing workflows, meaning we can safely assume a user will modify their routines if using old pkg/mnc generated output. I submitted this update because I couldn't use my established xmitgcm/xarray post-processing routines without considerable effort to make my mnc NetCDF files match MDS NetCDF files.

pkg/mnc creates output comparable to MDS+xmitgcm workflows, so there is a need to agree on how similar the output should be between these two post-processing routes. It looks like there was little emphasis on pkg/mnc output requiring to match MDS in the past. This PR starts to address "merging" output formatting between pkg/mnc and MDS+xmitgcm

[IvanaEscobar]

@IvanaEscobar I like this suggestion. It removes many unnecessary differences between MDS and MNC output. HFacC to hFacC makes total sense. With dxG to DXY and similar there's the complication that in the xmitgcm output, the variables are called dxG, rAz, etc. (and XG, inconsistently). Maybe that's something to discuss?

Probably, more people use xmitgcm (which is actually quite dormant currently), than the MNC output option, but I can't be sure.

I believe more users opt for an MDS+xmitgcm workflow. This PR drags mnc files to be more similar to MDS NetCDF files.

[mjlosch]

Hi all, I generated a table of (most) grid variables as they appear with MDSIO output, mic-output, and in xmitgcm-datasets:
grid_variable_list.csv
here as a pdf
grid_variable_list.pdf

@jm-c I also added variables as they appear in utils/matlab/load_grid.m.

Now we can fight, what's most consistent (o:

@IvanaEscobar Did you check, if tools like utils/python/MITgcmutils/scripts/gluemncbig, rdmnc, or utils/script/glumnc still work after changing the grid variable names for mnc output?

[IvanaEscobar]

My only input for choosing naming conventions will be that most users opt for MDS output, so it would be least inconvenient to switch to that naming convention universally.

@mjlosch I checked with utils/script/gluemnc and it works fine. The others are tools I don't tend to use.

[jm-c]

@mjlosch I looked at the pdf version of your table: may be could add an other column (the first one ?) with the list of grid variables as they appear in GRID.h ?

[mjlosch]

E voilà! The updated list of variables with a column that shows, what we can find in GRID.h:
grid_variable_list.pdf
grid_variable_list.csv

[IvanaEscobar]

Are these suggested names for grid parameters correct? I colored the ones we didn't discuss in yellow

OUTDATED grid_variable_list.pdf
OUTDATED grid_variable_list.csv

[jm-c]

@IvanaEscobar I looked at the new table, with the "Suggested names" new column. Here are some thought:

1. "angleCS" and "angleSN" are fine with me.
2. I would prefer to keep "rAw", "rAs" and "rAz" unchanged, and just rename "rA" to "rAc"
3. all new name for {x,y}{C,G} are good, including the {x,y}{U,V}, and also "rSurfC" and "rLowC"; and "rhoRef" is fine too.
4. Not very important, but I have a little issue with "Zl" and "Zu", because it has "r" units, so when running in P-coordinates, it will be in Pascal (Pa).
5. and regarding "U2zonDir" and "V2zonDir", it does not matter much because it's not used much, but could use the same upper/lower case writing as in GRID.h, "u2zonDir", "v2zonDir".

But if you have a strong preference for a particular name, could also stick with it.

[IvanaEscobar]

With your updates, here is the suggested name list:
grid_variable_list.csv
grid_variable_list.pdf

---

If this list looks right, the next step will be to correct pkg/mnc and update grid reading utilities.

[jm-c]

This looks all good to me, thanks @IvanaEscobar
And @mjlosch do you agree with new names ?

[mjlosch]

Thanks, this looks good to me.
Just to be sure: the column "SUGGESTED NAMES" will replace the column "MNC", and the other columns will be untouched (for now).

[IvanaEscobar]

@mjlosch I believe "SUGGESTED NAMES" will update pkg/mnc and expand upon the existing utils. Any additional naming updates can be addressed outside of this PR

[jm-c]

@IvanaEscobar Since we did not get any complain about the new names (in your updated table) , looks like we can move on to the next step, when you have find time.

[IvanaEscobar]

Done @jm-c

It looks like changing the grid names causes some tests to fail. What needs to change to fix the failing checks?

[mjlosch]

in pkg/mnc/mnc_cw_write_grid_info.F I found this:

      CALL MNC_CW_RS_W('R',fname,0,0,'XC',xC, myThid)
      CALL MNC_CW_RS_W('R',fname,0,0,'YC',yC, myThid)
      CALL MNC_CW_RS_W('R',fname,0,0,'XG',xG, myThid)
      CALL MNC_CW_RS_W('R',fname,0,0,'YG',yG, myThid)

There may be more places where XC etc needs to change to xC, but I really don't know.

[IvanaEscobar]

That was the fix needed. Thanks @mjlosch

I hadn't seen the additional subroutine under the stale MNC_CW_WRITE_GRID_INFO

[mjlosch]

I ran ./testreport -t isomip to generate some tiled mnc-output (tr_run.htd) and was able to glue the tiles grid.t*.nc into one global tile using both nco-based utils/script/gluemnc and python-based utils/python/MITgcmutils/scripts/gluemncbig. (I did have to remember how to do that and there are some requirements that my environments did not meet, oh well). It also works for other files, but there the new names are not used.

I am not aware of any python code in utils/python that makes explicit use of hardwired coordinate variable names.

[jm-c]

I pushed a little fix in load_grid.m (when used with just 2 arguments) but the changes in MNC grid-file variables look good to me.

Regarding python utils, should we make a little script there to show how to handle loading the old and the new MNC grid files ?

[IvanaEscobar]

@jm-c I'm not familiar with utils/MITgcmutils, but it looks like there is a script already there: mnc.py which provides rdmnc as a function.

Assuming MITgcmutils is included in your Python search path, a pseudo-script is:

import MITgcmutils

mymncFile = MITgcmutils.rdmnc("mnc_file_0001/state.0000000000.*")

Who would be the best contact for checking this utility?

[mjlosch]

I do not think it's necessary to have specific python script.

• In load_grid.m, the matlab function rdmnc.m load the content of the netcdf file(s) (here grid.*.nc) into structure S and then the fields in the structure are copied to matlab arrays, which then again are assembled into a structure.
• We never had a script like load_grid.m for python, just the function rdmnc with similar functionality as rdmnc.m. The python function returns a python dict of numpy arrays, as for rdmnc.m, this is independent of the names in the netcdf file, e.g.,

>> g=mit.rdmnc('grid.*.nc')
>> g.keys()
Out[21]: dict_keys(['Z', 'rC', 'Zp1', 'rF', 'Zu', 'rU', 'Zl', 'rL', 'drC', 'drF', 'X', 'Y', 'xC', 'yC', 'Xp1', 'Yp1', 'xG', 'yG', 'dxC', 'dyC', 'dxF', 'dyF', 'dxG', 'dyG', 'dxV', 'dyU', 'rAc', 'rAw', 'rAs', 'rAz', 'fCori', 'fCoriG', 'rLowC', 'rSurfC', 'Depth', 'hFacC', 'hFacW', 'hFacS'])

• xmitgcm does what load_grid.m does for MDS files (and more, however still not for tiled files): read all data from a directory and return the data in some single entity, here a xArray dataset.

@jm-c do you really think we need a python script/function like load_grid.m, i.e. where we load the grid either from MDS or NetCDF data, probably using the python functions rdmds and rdmnc?

[jm-c]

@mjlosch If we are sure that there is no user using python with MNC output then it's fine. Otherwise we should expect, after this renaming PR is merged, that someone will report about "why my analysis script don't work anymore", and we will need to explain how to look for name changes in, e.g., "dict" if using rdmnc, to rename/adapt the rest of the python analysis tool.
The same is true with matlab with the difference that we can point to an example (in this case, load_grid.m) on how to handle the name changes.

[mjlosch]

OK, I see the point, but would love to have @jahn's opinion. I wouldn't even know where within the utils/python directory we want to put such an example-script.

[jahn]

Not sure I understand what we are trying to illustrate and what load_grid.m does other than renaming the variables. Why not just read the grid netCDF files with rdmnc? And have a table somewhere with the old and new mnc names, maybe even in the doc string of rdmnc?

[IvanaEscobar]

Not an elegant solution, but there's now a utility that can be pointed to in the case that a user needs backwards compatibility and needs it in Python

Feel free to improve restore_MNC_keys

[IvanaEscobar]

The PR description was updated to account for backwards compatibility. Is this all we need to merge?

[jrscott]

Python routines in tutorial baroclinic gyre are hard-coded with mnc (old) names and should be fixed. There may be other places where the old names were hard-coded, will look further.

[jm-c]

I think the matlab version needs also some updates:

MITgcm/verification/tutorial_baroclinic_gyre/analysis/matlab_plots.m

Lines 20 to 45 in b7411f1

	% 1-D fields
	RC = ncread('grid.nc', 'RC'); % vertical grid, cell center locations
	drF = ncread('grid.nc', 'drF'); % vertical spacing of grid cells (thickness of cells)
	% 2-D fields (x,y)
	XC = ncread('grid.nc', 'XC'); % x-location of gridcell centers
	YC = ncread('grid.nc', 'YC'); % y-location of gridcell centers
	dyG = ncread('grid.nc', 'dyG'); % grid spacing in y-dim (separation between corners)
	rA = ncread('grid.nc', 'rA'); % surface area of gridcells
	% 3-D fields (x,y,z)
	HFacC = ncread('grid.nc', 'HFacC'); % vertical fraction of cell which is ocean
	% For convenience, load additional dimensional variables from
	% netcdf files (these variables are NOT included in binary output)
	% These grid variables are also present in netcdf diagnostic data files.
	% 1-D fields
	X = ncread('grid.nc', 'X'); % 1-D version of XC data
	Y = ncread('grid.nc', 'Y'); % 1-D version of YC data
	Xp1 = ncread('grid.nc', 'Xp1'); % x-location of lower left corner
	Yp1 = ncread('grid.nc', 'Yp1'); % y-location of lower left corner
	% Xp1 and Yp1 are effectively 1-D versions of grid variables XG,YG with
	% an extra data value at the eastern and northern ends of the domain.
	% See MITgcm users manual section 2.11 for additional MITgcm grid info
	% Number of gridcells in x,y, full domain:
	Nx = size(XC, 1);
	Ny = size(XC, 2);

[jm-c]

LGTM

[jm-c]

I took a quick look and just approved this PR changes. @mjlosch @jahn @jrscott do you have anything to add here ? otherwise, could think of merging this PR soon.

[jrscott]

I tested the (modified) matlab and two python routines in tutorial_baroclinic_gyre, all ran with no issues, LGTM

[mjlosch]

LGTM

[jahn]

LGTM

[jm-c]

Thanks everyone, will merge this PR later today.