JCSDA-internal / ufo-data

Tier-1 test files for ufo repository
1 stars 0 forks source link

Make variable names conform for ufo-data #312

Closed mo-keithstewart closed 1 year ago

mo-keithstewart commented 1 year ago

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_temperature cloud_area_fraction_in_atmosphere_layer >> cloud_volume_fraction_in_atmosphere_layer liquid_cloud_fraction >> liquid_cloud_volume_fraction_in_atmosphere_layer frozen_cloud_fraction >> ice_cloud_volume_fraction_in_atmosphere_layer

This 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.

mo-keithstewart commented 1 year ago

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.

mikecooke77 commented 1 year ago

@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 cloud_area_fraction_in_atmosphere_layer >> cloud_volume_fraction_in_atmosphere_layer

BenjaminTJohnson commented 1 year ago

@mikecooke77 I potentially disagree with these changes:

cloud_area_fraction_in_atmosphere_layer >> cloud_volume_fraction_in_atmosphere_layer
liquid_cloud_fraction >> liquid_cloud_volume_fraction_in_atmosphere_layer
frozen_cloud_fraction >> ice_cloud_volume_fraction_in_atmosphere_layer

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 (kg/m^2).

I can be convinced on this.

mikecooke77 commented 1 year ago

@mikecooke77 I potentially disagree with these changes:

cloud_area_fraction_in_atmosphere_layer >> cloud_volume_fraction_in_atmosphere_layer
liquid_cloud_fraction >> liquid_cloud_volume_fraction_in_atmosphere_layer
frozen_cloud_fraction >> ice_cloud_volume_fraction_in_atmosphere_layer

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 (kg/m^2).

I can be convinced on this.

I thought this was the case. In which case we should have both: cloud_area_fraction_in_atmosphere_layer and cloud_volume_fraction_in_atmosphere_layer. The same for the other two e.g. liquid_cloud_area_fraction_in_atmosphere_layer and liquid_cloud_volume_fraction_in_atmosphere_layer ice_cloud_area_fraction_in_atmosphere_layer and ice_cloud_volume_fraction_in_atmosphere_layer @jameshocking-mo which is most accurate from an RTTOV point of view?

jameshocking-mo commented 1 year ago

@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.

ss421 commented 1 year ago

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 lfric-lite-jedi and I think @stemiglio added them.

To clarify, the suggestion is to have the following:

cloud_area_fraction_in_atmosphere_layer [stash code 265] cloud_volume_fraction_in_atmosphere_layer [stash code 266] liquid_cloud_fraction (and not liquid_cloud_volume_fraction_in_atmosphere_layer) [stash code 267] frozen_cloud_fraction (and not ice_cloud_volume_fraction_in_atmosphere_layer) [stash code 268]

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 cloud_volume_fraction_in_atmosphere_layer but I'm not sure what to do about the other geoval data.

(some further discussion on the cloud_area_fraction_in_atmosphere_layer and cloud_volume_fraction_in_atmosphere_layer is given in https://github.com/JCSDA-internal/ufo/issues/2627.

mikecooke77 commented 1 year ago

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.

mikecooke77 commented 1 year ago

cloud_area_fraction_in_atmosphere_layer [stash code 265] > 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 cloud_volume_fraction_in_atmosphere_layer but I'm not sure what to do about the other geoval data.

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.

ss421 commented 1 year ago

If we are adopting cloud_volume_fraction_in_atmosphere_layer in the interface then we need to make the RTTOV change as part of this I think?

In terms of the ufo data - we leave the ufo data as it is but create another one for cloud_volume_fraction_in_atmosphere_layer? Who would be best placed to advise on how to do that?

mikecooke77 commented 1 year ago

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.

mikecooke77 commented 1 year ago

I think this is also an issue for: liquid_cloud_volume_fraction_in_atmosphere_layer ice_cloud_volume_fraction_in_atmosphere_layer

ss421 commented 1 year ago

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.

Thanks @mo-keithstewart said that he has setup a meet - thanks.

ss421 commented 1 year ago

I think this is also an issue for: liquid_cloud_volume_fraction_in_atmosphere_layer ice_cloud_volume_fraction_in_atmosphere_layer

What is the issue? My reading from this PR is that we should not change the name, so we will have:

liquid_cloud_fraction (and not liquid_cloud_volume_fraction_in_atmosphere_layer) [stash code 267] frozen_cloud_fraction (and not ice_cloud_volume_fraction_in_atmosphere_layer) [stash code 268]

Is that not correct?

mikecooke77 commented 1 year ago

I think this is also an issue for: liquid_cloud_volume_fraction_in_atmosphere_layer ice_cloud_volume_fraction_in_atmosphere_layer

What is the issue? My reading from this PR is that we should not change the name, so we will have:

liquid_cloud_fraction (and not liquid_cloud_volume_fraction_in_atmosphere_layer) [stash code 267] frozen_cloud_fraction (and not ice_cloud_volume_fraction_in_atmosphere_layer) [stash code 268]

Is that not correct?

I think the um and lfric mapping is correct. However for CRTM they will need liquid_cloud_area_fraction_in_atmosphere_layer and ice_cloud_area_fraction_in_atmosphere_layer. So in ufo it makes sense to map the current names to area and then its a smaller change for CRTM. New volume equivalents will be add in the ufo_variables_mod and then RTTOV would use those for now.

mo-keithstewart commented 1 year ago

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.

mikecooke77 commented 1 year ago

Some geovals files end in .nc, others end in .nc4. I think it should be made consistent.

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.