Skip to content
This repository has been archived by the owner on Oct 23, 2020. It is now read-only.

Inconsistent names for region file output from AMs #1305

Open
q-rai opened this issue May 2, 2017 · 10 comments
Open

Inconsistent names for region file output from AMs #1305

q-rai opened this issue May 2, 2017 · 10 comments

Comments

@q-rai
Copy link

q-rai commented May 2, 2017

Quick history: @DieMuhkuh (Nils) implemented Meridional Overturning Circulation (MOC), including region file capability. I implemented region file capability for Meridional Heat Transport (MHT) and Water Mass Census (WMC), using the same naming scheme for variables as him, to make things consistent between AMs. Little did we know that the same naming scheme had already been used in a different context for hard-wired region support in Surface Area Weighted Averages (SAWA), Layer Volume Weighted Averages (LVWA), and possibly some more.

Right now we have the following:
MOC, MHT and WMC use "Region" as a suffix to all region mask file based output. Variables with no suffix are global computations (or in case of WMC, use hard-wired regions).
Some AMs with hard-wired regions use "Region" as a suffix for hard-wired region based output. For region file based output, I used "RF" (short for region file) as a suffix to differentiate between the two.

I see multiple possible solutions:

  • I propose refactoring MOC and MHT to use "RF" instead of "Region" as a suffix. This will keep all analysis scripts for the original (global and hard-wired region) AMs intact. I assume there is not a lot of simulation output using region file versions of MOC and MHT yet.
  • Refactor SAWA and LVWA to remove "Region" from their original variable names. This would break old analysis scripts.
  • Leave names as they are but remain aware of this difference. If/when MPAS-O migrates to always using region files, the names could be made consistent at that time.

I don't have a preference, I just wanted to make everyone aware of these differences to avoid confusion later.

@q-rai
Copy link
Author

q-rai commented May 2, 2017

@milenaveneziani You're the most likely person to have used MOC and MHT regions. What do you think?

@milenaveneziani
Copy link

Hi @q-rai, thanks for bringing this up. I am a bit confused by what you say about the AMs with hard-wired regions: I thought they did not use variables with suffixes, but just one array with region index as one of the dimensions. Is that incorrect?

@q-rai
Copy link
Author

q-rai commented May 2, 2017

@milenaveneziani You're correct about the arrays, it's all in one with region index as one of the dimensions. The array name typically ends on "Region", e.g. "minValueWithinOceanRegion" im SAWA.
I just realized that WMC actually falls within the same category as MHT and MOC when it comes to naming (but not how it works under the hood). I'll fix my post to reflect this! (done)

@milenaveneziani
Copy link

ok, thanks. So, what you were saying above about the 'RF' suffix refers to the output file? MOC regional is saved to a different file at the moment?

As far as name conventions go, I generally prefer to keep the names longer rather than shorter, so that I know exactly what they refer to. Thus I would prefer 'Region' over 'RF'. But again this is just my preference.

@milenaveneziani
Copy link

I talked with @q-rai in person and now I understand the details of this issue.
I suggested to use a different suffix than RF, something like 'RegionFromMask', but if other people have other suggestions please let us know.

@philipwjones
Copy link
Contributor

My only opinion is that it be short. Some of these names are already Faulknerian in length. While I appreciate the attempt to have descriptive names, I would advocate just a simple "Region" (or even "Reg") suffix.

@q-rai
Copy link
Author

q-rai commented May 4, 2017

How about:

  • for the hard-wired regions (which include 'global') xyzRegion -> xyz
  • for the region files across all AMs abcRF -> abcReg, or abcRegion -> abcReg

That way, naming is consistent between, and we don't have old runs using xyzRegion for hard-wired regions conflicting with new runs using abcRegion for region files.

@milenaveneziani
Copy link

I would probably leave 'Region' for the old variables, to avoid confusion. For the new variables, how about trading RF with RegionMask? it's only 4 more letters..
So, say, minValueWithinOceanRegion would have the corresponding minValueWithinOceanRegionMask.
(ps: I do agree some variables are way too long)

@q-rai
Copy link
Author

q-rai commented May 10, 2017

I've changed the names in SurfaceAreaWeightedAverages and LayerVolumeWeightedAverages like you suggested, @milenaveneziani (currently only on my github but I'll push everything by tomorrow). If I have time, I'll do MHT, MOC and WaterMassCensus, but I'll give testing those first two priority.

@q-rai
Copy link
Author

q-rai commented May 25, 2017

Update: I did not have time to update the names in MHT, MOC, and WMC before I left LANL, so these still need adapting.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants