Repository navigation
Make variable names conform for ufo-data - #312
Conversation
|
Note: all ctests are passing on SPICE and CRAY_GNU. However there is an issue with the make process on CRAY_INTEL and CRAY_INTEL_DEBUG. I have contacted Matt Shin about this and he is investigating. |
|
@BenjaminTJohnson there are quite a few files associated with CRTM that are changed as part of this PR. Do you think these are sensible changes? I'm thinking in particular about |
|
@mikecooke77 I potentially disagree with these changes: Strictly speaking, these are not volume fractions, but area fractions. I suppose one could consider these as volume fractions, but it's not precisely true. We provide water "content" as a layer water path ( I can be convinced on this. |
I thought this was the case. In which case we should have both: |
|
@mikecooke77 As a bit of an outsider, it's not obvious to me that it's a good idea to rename volume fractions as area fractions, or vice versa, since the two are different. Having discussed this with Cyril Morcrette in the past, the area fraction is most appropriate for use with RTTOV, but in its absence bulk/volume fraction can be used. |
mikecooke77
left a comment
There was a problem hiding this comment.
Thanks @jameshocking-mo and @BenjaminTJohnson for the comments. @mo-keithstewart It looks like we shouldn't just be renaming these variables. My suggestion is we have both area and volume fractions for the cloud variables. This will require both to be in the ufo_variables_mod and these should not be being renamed in this ufo-data PR. It will be down to the individual observation operators to decide which one to use. This clearly has implications for all linked PRs.
|
The names adopted here are documented in the excel spreadsheet which I can share on teams. Here we have adopted the agreed names in that sheet. That was based on conversations with people in the Met Office and we didn't get any objections. The proposed names were originally included in To clarify, the suggestion is to have the following:
Is that correct - if so I can update the spreadsheet. It is my understanding that RTTOV uses the field associated with stash code 266. We do not output stash code 265 in our operational configuration. If we adopted two names as suggested above, how do we create the ufo geoval data that is required for the additional variable? Who created the data to begin with? If it is us then it makes sense to rename the data to (some further discussion on the |
|
If you look at the final comments on https://github.com/JCSDA-internal/ufo/issues/2627. There was the suggested need for two variables one called area and one called volume. If we stick with what is currently in ufo_variables_mod and add new variables for volume that does not affect those using CRTM and those associated model interfaces. The change as is will affect all model interfaces outside of the Met Office. We will have to make some changes in RTTOV but these can be done shortly after these changes or as part of it. |
To answer this question specifically. We would use volume from the model interfaces and we would not create area. The RTTOV interface will need to change to expect the new variable name. Other model interfaces would still produce area values for use with CRTM. |
|
If we are adopting In terms of the ufo data - we leave the ufo data as it is but create another one for |
|
For the testing the RTTOV geoval file can just be renamed because area and volume fraction are very similar values. The CRTM files will need to be reverted. As you say the RTTOV interface will need changing and I'm happy to discuss this with Keith. This will also affect the other associated PRs in other ways such-as opsinputs. |
|
I think this is also an issue for: |
Thanks @mo-keithstewart said that he has setup a meet - thanks. |
What is the issue? My reading from this PR is that we should not change the name, so we will have:
Is that not correct? |
BenjaminTJohnson
left a comment
There was a problem hiding this comment.
Some geovals files end in .nc, others end in .nc4. I think it should be made consistent.
I think the um and lfric mapping is correct. However for CRTM they will need |
…iable name changes.
|
I have added in the changes created by Mike that affect two of the data files, which required to be reverted back to their original versions. |
I agree this would be good to do. I think it will probably require a separate PR and a decision to be made on a naming convention. |
mikecooke77
left a comment
There was a problem hiding this comment.
Changes look good to me.
PatNichols
left a comment
There was a problem hiding this comment.
Thanks. It looks good.
Description
This is the work required for issue #311 for changes to the variable names so that there is more consistency in the system, as defined in this spreadsheet
The following fields are updated:
theta>>potential_temperaturecloud_area_fraction_in_atmosphere_layer>>cloud_volume_fraction_in_atmosphere_layerliquid_cloud_fraction>>liquid_cloud_volume_fraction_in_atmosphere_layerfrozen_cloud_fraction>>ice_cloud_volume_fraction_in_atmosphere_layerThis fixes an issue we currently have with variable mismatch between different parts of the system.
(NB: two of the data files, abi_g16_geoval_2018041500_m_qc.nc4 and pace_radiance_geovals_2019032112.nc, have been reverted to their original versions to retain the variable
cloud_area_fraction_in_atmosphere_layer)Acceptance Criteria (Definition of Done)
Test builds and run
Test output is here.
Dependencies
Requires changes to ops-um-jedi, opsinputs, ufo, lfric-lite-jedi, saber, vader and sith.