search

JCSDA-internal / ioda-converters

Various converters for getting obs data in and out of IODA
9 stars 4 forks source link

OMPS O3 TC converter for JCSDA ingest #1312

Closed jeromebarre closed 1 year ago

jeromebarre commented 1 year ago

Description

Creation of a simplified OMPS O3 NM converter to fit the JCSDA ingest needs. The already existing converter might just fit the needs for GMAO. Instead of modifying the old converter, I created a new much simpler one. But I am not sure if ompsnm_o3_nc2ioda.py is being used at all? @kwargan let us know, please. If not, we should probably remove it.

Output files have been tested with fv3-jedi and the column operator, hofx returns correct values.

Issue(s) addressed

https://github.com/JCSDA-internal/AOP23/issues/147 https://github.com/JCSDA-internal/ioda-converters/issues/1308

Dependencies

No dependencies as the format is equivalent to what is in UFO, so no need to create new tests there.

Impact

Checklist

kwargan commented 1 year ago

Sorry, I honestly don't know if ompsnm_o3_nc2ioda.py has been used or if there's any need for it. @karpob may know

karpob commented 1 year ago

@kwargan It all depends one what you mean by use. As in we use it currently, no. If we're talking about a world without the GSI as a preprocessor and without your preprocessing, then we'll need some of the information I transferred looking at the code you wrote that does QC.

jeromebarre commented 1 year ago

I guess this is the QC part you are talking about. https://github.com/JCSDA-internal/ioda-converters/blob/eb264e1fa14818493ea4a6bd988fdd7136633b5c/src/compo/ompsnm_o3_nc2ioda.py#L128-L131 @karpob Is it wise to unmask those?

karpob commented 1 year ago

It's difficult to say why I was playing with the mask there on a script I wrote quite some time ago, it probably wasn't for my own edification. I think it was to match the output of a fortran code we use as a pre-processor which ignores the mask, and to keep dimensions of arrays the same, so I could avoid a loop. Our concept in general was not to necessarily do QC within the ioda converter, but carry any flags or information we'd need to do QC later on.

nancylbaker commented 1 year ago

FYI: Navy has our own converter based on the NAVGEM decoder/pre-processor only reformats the data.

We also don't assimilate the total column product, but do assimilate the nadir profile product. This PR is just for the total column.

jeromebarre commented 1 year ago

FYI: Navy has our own converter based on the NAVGEM decoder/pre-processor only reformats the data.

We also don't assimilate the total column product, but do assimilate the nadir profile product. This PR is just for the total column.

The profile PR will come in August when I return from vacation and will use the averaging kernels.

jeromebarre commented 1 year ago

It's difficult to say why I was playing with the mask there on a script I wrote quite some time ago, it probably wasn't for my own edification. I think it was to match the output of a fortran code we use as a pre-processor which ignores the mask, and to keep dimensions of arrays the same, so I could avoid a loop. Our concept in general was not to necessarily do QC within the ioda converter, but carry any flags or information we'd need to do QC later on.

To keep the dimension the same I use the opposite logic which is to apply the mask to unmasked arrays: https://github.com/JCSDA-internal/ioda-converters/blob/c9f33a0483c108b68ce7fa6a6ce073876226cbf9/src/compo/omps_o3_nm_h52ioda.py#L98-L105 seems safer maybe?

karpob commented 1 year ago

I suppose it could be. It's more an issue of having additional logic to handle the fill values at some point.

nancylbaker commented 1 year ago

Navy also uses the kernel method for OMPS NP.

jeromebarre commented 1 year ago

Navy also uses the kernel method for OMPS NP.

What UFO operator are you using for this? As far as I know only columnreterieval can use averaging kernels. If you are already using it great if you are using something else I'd like to know if possible. Thanks

jeromebarre commented 1 year ago

Given the comments above on the other OMPS NM converter (not really being used and being GSI-specific), we should go ahead and review this PR and merge it. The fate of this other converter could be decided later.

PatNichols commented 1 year ago

Given the comments above on the other OMPS NM converter (not really being used and being GSI-specific), we should go ahead and review this PR and merge it. The fate of this other converter could be decided later.

Agreed.