Remove SOCA internal variable names #1082
Conversation
|
quick couple questions:
|
The CI tests do not run
just the variable names of the atlas fields are changing. |
| getval name: sea_ice_category_area_fraction | ||
| getval name surface: sea_ice_area_fraction # note: not accurate, should be "sum" not "surface" | ||
| - name: sea_ice_category_area_fraction | ||
| name surface: sea_ice_area_fraction # note: not accurate, should be "sum" not "surface" |
There was a problem hiding this comment.
outdated comment
There was a problem hiding this comment.
nope, still accurate. sea_ice_area_fraction is a summation of the category variable
| Valid time: 2018-04-15T00:00:00Z | ||
| ssh min=-1.9244847628277935 max=0.9272826517867588 mean=-0.2767903423591662 | ||
| tocn min=-1.8883899372702533 max=31.7004645720658580 mean=6.0175649583533595 | ||
| socn min=10.7210460395083924 max=40.4416591897031168 mean=34.5443897261524953 | ||
| hocn min=0.0009999999999977 max=1345.6400000000003274 mean=128.6280642065023017 | ||
| layer_depth min=2.2854716757984130 max=5658.3057467114012979 mean=1200.5229536158392420 | ||
| mld min=2.2854716757984130 max=4593.1533423819937525 mean=192.4109073940401515 | ||
| Output increment: | ||
| Output increment: |
There was a problem hiding this comment.
why no name change here?
Also, unrelated to his PR, but we've got a nan in the reference output ... Oops.
There was a problem hiding this comment.
I wanted to do a PR where none (or as few as possible) of the answers change so that I know the PR was correct. The test references only check numbers, not the text, so everything still works with the old variable names. At some point, I'll regenerate all the references (but that would mean all the numbers would slightly change, I didn't want to deal with that right now)
There was a problem hiding this comment.
also, that nan is a "feature", at least that's what I'm telling myself. It indicates a mismatch between the low and high-resolution grid, so there exist valid ocean points in one grid that don't exist in the other, so the interpolation produces a nan for the grid cell. In the real world, we would take the time to make sure the source and target grids line up better.
Description
This PR removes the field variable names internal to SOCA, and instead adopts the proper model interface names throughout the code.
For example, the temperature field uses three different variable names depending on where it is in the code:
Temptocnsea_water_potential_temperaturetocnis now removed. This was done for all variables.Note
The model interface variable names were not changed (with the exception of
mld) because the official CCPP names we want to adopt do not exist yet for most of the ocean variables (with the exception ofmld, which I've changed to the CCPP standardocean_mixed_layer_thickness)Why?
this will make future developments in SABER and VADER easier. For example, this PR (https://github.com/JCSDA-internal/vader/pull/208) has been languishing but should be easier to adopt within soca after this PR (In short, it's hard to use vader with soca the way it is right now because vader recipes require the ingredients fields to have specific names, but within soca I would have to first copy all those variables over before calling vader. The linear variable change was even more annoying because I would have to then do it again in the other direction. It was a mess)
This might also make refactoring ice variable transforms easier, as well as future re-implementation of the u/v destaggering and rotating
Impact
File IO will still be the same, so the only places you should need to make any changes:
test/Data/fields_metadata.ymlfile.Testing
I made sure that the ctest reference answers did not change, with the exception of the state test (the analytic state random values depend on the variable name, which of course changed).
build-group=https://github.com/JCSDA-internal/coupling/pull/48