UKMO rename qcl and qcf

mikecooke77 commented 1 month ago

Description

JEDI names for Met Office State/Increment variables currently include:

specific_humidity mass_content_of_cloud_ice_in_atmosphere_layer mass_content_of_cloud_liquid_water_in_atmosphere_layer qrain qsat

The two mass contents names are wrong, and used (correctly) as var_clw_wp and var_cli_wp in UFO by NOAA.

Choice of new temporary names

We propose to rename the 2 wrong names as specific_cloud_ice and specific_cloud_liquid_water, for consistency with specific_humidity, without waiting for a full renaming code sprint. Hopefully, this will be later renamed to more explicit CCPP names (mixing ratios wrt moist air and condensed water).

Issue(s) addressed

Renaming of water variables · Issue #521 · JCSDA-internal/saber (github.com) (about the inconsistency with qrain)

Confusion between cloud mixing ratio and liquid/ice water paths · Issue #3217 · JCSDA-internal/ufo (github.com) (about the same name being used for two things by NOAA and us, correctly by NOAA and wrongly by us)

What mixing ratio variables does RTTOV need? · Issue #3286 · JCSDA-internal/ufo (github.com) (About the fact that even independently of the naming, there is a scientific issue in the inputs we give to RTTOV)

Dependencies

List the other PRs to be merged at the same time

build-group=JCSDA-internal/ufo#3424 build-group=JCSDA-internal/ufo-data#436

for integration tests

JCSDA-internal/vader/pull/270
JCSDA-internal/saber/pull/916
JCSDA-internal/ops-um-jedi/pull/229
MetOffice/jjtests/pull/36
JCSDA-internal/lfric-jedi/pull/856
MetOffice/opsinputs/pull/220
JCSDA-internal/jedi-docs/pull/795
MetOffice/jjdocs/pull/200

UKMO specific testing

Impact

The dependent directories all need to be merged at the same time.

Checklist

[x] I have performed a self-review of my own code
[x] I have made corresponding changes to the documentation - PR
[x] I have run the unit tests before creating the PR

yaswant commented 1 month ago

We never implemented integration test in opsinputs. I can add this in this PR if you wish. Note that the integration tests will not run in JCSDA containers.

Also I am not sure, about the following build groups

build-group=https://github.com/MetOffice/opsinputs/pull/220
build-group=https://github.com/JCSDA-internal/jedi-docs/pull/795
build-group=https://github.com/MetOffice/jjdocs/pull/200

ctgh commented 1 month ago

Thanks for coordinating this large change. Do you need to change any of the names in src/opsinputs/opsinputs_cxfields_mod.F90?

mikecooke77 commented 1 month ago

We never implemented integration test in opsinputs. I can add this in this PR if you wish. Note that the integration tests will not run in JCSDA containers.

Also I am not sure, about the following build groups
build-group=https://github.com/MetOffice/opsinputs/pull/220
build-group=https://github.com/JCSDA-internal/jedi-docs/pull/795
build-group=https://github.com/MetOffice/jjdocs/pull/200

I know they won't be picked up but it makes the PRs much easier if I can just copy and paste to each of the PRs. It doesn't seem to affect the PRs and keeps things consistent.

mikecooke77 commented 1 month ago

Thanks for coordinating this large change. Do you need to change any of the names in src/opsinputs/opsinputs_cxfields_mod.F90?

No because they are already using short names from the ufo_variables module e.g.: https://github.com/MetOffice/opsinputs/blob/develop/src/opsinputs/opsinputs_cxfields_mod.F90#L81-L82

@MayeulDestouches had already separated the MO and other names in a previous PR in ufo.

mikecooke77 commented 1 month ago

Note that the integration tests will not run in JCSDA containers.

@yaswant could you clarify why this is the case?

yaswant commented 1 month ago

Note that the integration tests will not run in JCSDA containers.

@yaswant could you clarify why this is the case?

opsinputs is public and currently the automated CI run in JCSDA container environment in GitHub standard runner.

These containers dont have JEDI-LFRic library, and
GitHub free runners are dual core, so I doubt we can run the integration tests there.

There is a workflow to trigger manually via GitHub action that runs on our self-hosted runner - I can amend that to include full integration tests.

ctgh commented 1 month ago

Rerunning the generate_unittest_netcdfs.py script often causes all of the files to be updated. As seen in this PR, this unit tests still pass, so it must be a subtle change. I guess it could be to do with a change in the version of python, or the underlying netCDF library, but it seems unlikely that would change so often. I have never tried using nccmp or similar to get to the bottom of it.

ctgh commented 1 month ago

I just tried regenerating all of the files and then comparing the before and after versions of one of them. The only difference was in the global attribute:

DIFFER : VALUES OF GLOBAL ATTRIBUTE : _NCProperties : version=2,netcdf=4.9.2,hdf5=1.14.3 <> version=2,netcdf=4.9.0,hdf5=1.14.0

It does look like a small version change.

MetOffice / opsinputs