JCSDA-internal / ioda-converters

Various converters for getting obs data in and out of IODA
8 stars 2 forks source link

imsfv3_scf2ioda.py not updated for latest IODA format #1479

Open ClaraDraper-NOAA opened 4 months ago

ClaraDraper-NOAA commented 4 months ago

Description

The converter for IMS-derived snow cover has not been updated to the latest IODA converter.

When I try to assimilate data using this converter, JEDI reports that there are no obs in the file. When I look at the converter, it looks like it hasn't been updated to the latest IODA version (e.g., has height instead of stationElevation; has snow depth in units if m, not mm). I'm not sure why JEDI is reporting zero obs, but am assuming updating the IODA version will correct that.

Requirements

This is a bugfix.

Success is JEDI LETKF recognising that there are obs in the converted file.

Acceptance Criteria (Definition of Done)

Clara's offline land DA test passes.

Dependencies

No dependencies

ClaraDraper-NOAA commented 4 months ago

@CoryMartin-NOAA @jiaruidong2017 Are you successfully assimilating the IMS obs in GDASApp?

jiaruidong2017 commented 4 months ago

@ClaraDraper-NOAA we don't have the unit test to assimilate the IMS obs in GDASApp. The unit test for the IMS obs is the ioda converter.

ClaraDraper-NOAA commented 4 months ago

I've sorted out the issue with my IMS DA, however the issue remains that the IMS converter in this repo needs to be updated (units m -> mm, height-> stationElevation). There are also some changes that need to be made to this converter (obs error, obs time).

There are tests in this repo for the converters themselves, but not for the DA updates of the output (hence, the converters have become out-dated). I suggest we need to add some tests (or amend the current letkf snow DA test) in fv3-jedi to catch this issue. @jiaruidong2017 has had the same problem with the BUFR snow obs.

YoulongXia-NOAA commented 4 months ago

@ClaraDraper-NOAA, I am being requested by EMC management to re-run nldas2 from May 2022 to present time as the first priority task. After I complete that task, I will discuss with you and then update this converter based on some detailed guidance. Thank you for your patience.

YoulongXia-NOAA commented 4 months ago

@ClaraDraper-NOAA, for the update, the converter needs to do: units m -> mm, height-> stationElevation, besides these chnages, could you please give me some explanation about "There are also some changes that need to be made to this converter (obs error, obs time)"? If you have an example on Hera, could you please give me the directory so that I can take a look? Thank you.

BenjaminRuston commented 4 months ago

@ClaraDraper-NOAA thanks for isolating this

have you tried the ioda updaters on a file and do they work? they use this: https://github.com/JCSDA-internal/ioda/blob/develop/share/ioda/yaml/validation/ObsSpace.yaml

it has lists of names, with the first entry being the current standard and updaters will rename other name to the leading one. the updaters will also do date_time to dateTime (integer seconds from an epoch) conversion

ClaraDraper-NOAA commented 4 months ago

@BenjaminRuston I can convert the files that we have easily enough. Let's discuss at our next meeting how to prevent this happening going forward.

@YoulongXia-NOAA I have some science changes for the IMS converter. I'll start a separate issue for those.

BenjaminRuston commented 3 months ago

@ClaraDraper-NOAA added some labeling so this stays on our boards

will try to get to this shortly ;0)

BenjaminRuston commented 3 months ago

@CoryMartin-NOAA regarding how to trap this, the WARNINGS regarding convention are not trapped (though they could be). This is another technical debt, potentially sprint issue. But the mechanism to report these is there, would be good to start picking away at these and then enable the strict enforcement of the conventions

ClaraDraper-NOAA commented 3 months ago

@BenjaminRuston From the user end, I really like the warning option. It'll allow us to see what the problem is immediately when we try to use an old converter, without committing anyone to updating converters that no one is using.

FYI: Youlong, who will work on this is on leave until early April. Once he's back, we'll get started on updating this converter, and also making some small science changes.

BenjaminRuston commented 3 months ago

the warning option is already in place, but you have to look at the log for the individual converter