JCSDA-internal / ioda-converters

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

Hotfix to add releaseTime to adpupa converter #1328

Closed haydenlj closed 1 year ago

haydenlj commented 1 year ago

Description

Temporary hotfix to add releaseTime to files output by the new adpupa prepbufr to ioda converter so that output files can be used by Skylab. Can be reverted once the planned more robust additions have been made

Issue(s) addressed

Resolves #1322

Dependencies

none

Impact

Output IODA files can be used in Skylab

Checklist

haydenlj commented 1 year ago

I don't believe this is what you want... The reason is that now you've mixed your data set with fields that are grouped by prepbufrDataLevelCategory (latitude, longitude, stationElevation... and so on) with things that are not grouped (timeOffset, specificHumidity)... The problem here is that you've lost your ability to correlate between these fields (for example you can no longer tell how latitude, longitude lines up with specific humidity). You have to be consistent with grouping to make datasets that make sense.

@rmclaren Ah ok, in that case I misunderstand what the bufr get function is doing. So it is sorting by prepbufrDataLevelCategory rather than just flattening along that axis?

rmclaren commented 1 year ago

@haydenlj The relative order stays the same (its not due to sorting). When you group a field what you are doing is reshaping the array. If you don't reshape your arrays in a consistent way then data in those arrays won't line up in the way you expect.

BenjaminRuston commented 1 year ago

let's not merge this until we have NOAA blessing that it's working for them as well

gthompsnJCSDA commented 1 year ago

@haydenlj Could you also try running the ioda-validate.x program on the output file that gets created? That will help ensure we also don't have something like MetaData/height as an integer when it should be a float. Or plenty of other possible examples as well. This just seems like the right time to catch anything we can that could become technical debt later.

BenjaminRuston commented 1 year ago

we do not think there is any grouping issue and we would like to have NOAA EMC test this PR to see if it still reproduces the answer you expect.

@emilyhcliu please advise

PraveenKumar-NOAA commented 1 year ago

we do not think there is any grouping issue and we would like to have NOAA EMC test this PR to see if it still reproduces the answer you expect.

@emilyhcliu please advise

@BenjaminRuston I will be happy to test this out.

PraveenKumar-NOAA commented 1 year ago

we do not think there is any grouping issue and we would like to have NOAA EMC test this PR to see if it still reproduces the answer you expect.

@emilyhcliu please advise

@BenjaminRuston I tested this PR/ Python script and it is reproducing the expected results. If @emilyhcliu @rmclaren are happy with this new format, I will approve this PR.

PraveenKumar-NOAA commented 1 year ago

@haydenlj Could you also try running the ioda-validate.x program on the output file that gets created? That will help ensure we also don't have something like MetaData/height as an integer when it should be a float. Or plenty of other possible examples as well. This just seems like the right time to catch anything we can that could become technical debt later.

@gthompsnJCSDA I ran ioda-validate.x program on the output file and there were a few errors/ warnings in the final results. A few new prepBUFR variables like prepbufrDataLevelCategory (CAT) and temperatureEventCode (TPC) also need to be added to the ObsSpace.yaml. I will create an issue for this.

gthompsnJCSDA commented 1 year ago

@haydenlj Could you also try running the ioda-validate.x program on the output file that gets created? That will help ensure we also don't have something like MetaData/height as an integer when it should be a float. Or plenty of other possible examples as well. This just seems like the right time to catch anything we can that could become technical debt later.

@gthompsnJCSDA I ran ioda-validate.x program on the output file and there were a few errors/ warnings in the final results. A few new prepBUFR variables like prepbufrDataLevelCategory (CAT) and temperatureEventCode (TPC) also need to be added to the ObsSpace.yaml. I will create an issue for this.

It seems we must already have something similar to DataLevelCategory - could you try to re-use something that is similar, such as verticalSignificance? And, isn't temperatureEvent already in the ObsSpace.yaml and was already exactly designed for radiosonde data I thought (similar to windEvent and moistureEvent, etc.).

BenjaminRuston commented 1 year ago

@emilyhcliu looks like @PraveenKumar-NOAA has tested and happy with result are you ready to move forward

PraveenKumar-NOAA commented 1 year ago

@gthompsnJCSDA thanks, there is no need to add any extra variables to the ObsSpace.yaml for the adpupa prepbufr.

@haydenlj please use ObsValue/verticalSignificance for MetaData/prepbufrDataLevelCategory (CAT) and QCFlags/qualityFlags for the MetaData/temperatureEventCode (TPC), then there will be no error in your ioda-validate.x run, you just need to take care of a few warnings. I can do that easily.

haydenlj commented 1 year ago

QCFlags/qualityFlags

Ok thanks @PraveenKumar-NOAA I updated the names

BenjaminRuston commented 1 year ago

@haydenlj looks like the testoutput may need to be updated:

Write data to variables
DIFFER : NUMBER OF GROUPS : 4 ("/") <> 3 ("/")
DIFFER : VARIABLE : prepbufrDataLevelCategory : DOES NOT EXIST IN "testrun/prepbufr_adpupa_api.nc"
DIFFER : VARIABLE : temperatureEventCode : DOES NOT EXIST IN "testrun/prepbufr_adpupa_api.nc"
DIFFER : VARIABLE : verticalSignificance : DOES NOT EXIST IN "testoutput/prepbufr_adpupa_api.nc"
DIFFER : Failed to find group /QCFlags in file "testoutput/prepbufr_adpupa_api.nc"
emilyhcliu commented 1 year ago

@haydenlj Could you give me the definition of the releaseTime? Is it the earliest time from dateTime of each profile?