JCSDA-internal / ioda-converters

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

Add a new variable for the wigosid to the bufr2ioda code(s): new #1407

Closed PraveenKumar-NOAA closed 9 months ago

PraveenKumar-NOAA commented 10 months ago

Description

This PR adds a new Wigos variable like dateTime to the bufr2ioda code.

The following files are new:

The following files have been changed to add the functionality:

The wigos variable is look like this: WGOSIDS-WGOSISID-WGOSISNM-WGOSLID, e.g. 0-20000-0-71941

where the naming convention is stationWIGOSId (already exist in the ObsSpace.yaml).

The ioda-validate.x is also ran over the output NetCDF file and the results are: Final results: errors: 0 warnings: 0

Issue(s) addressed

Resolves #1402

Dependencies

bufr2ioda

Impact

None

Checklist

PraveenKumar-NOAA commented 9 months ago

Thanks for adding this! I have a couple minor comments. Also are there corresponding updates that need to be made in the ioda ObsSpace.yaml? Thanks!

PraveenKumar-NOAA commented 9 months ago

Thanks for adding this! I have a couple minor comments. Also are there corresponding updates that need to be made in the ioda ObsSpace.yaml? Thanks!

@srherbener thanks, no we don't need to make any updates int the ioda ObsSpace.yaml. I am sent you an email about 10 days ago about this.

BenjaminRuston commented 9 months ago

coding norms and this ctest not passing: test_iodaconv_bufr_ncep_wigos

gthompsnJCSDA commented 9 months ago

Praveen, thanks for getting around to this long icebox issue that I wrote. Yup, you went the route I think was best to construct a new string out of a series of 4 grouped mixed items. Once you get the ctests passing, I will approve as well.

PraveenKumar-NOAA commented 9 months ago

coding norms and this ctest not passing: test_iodaconv_bufr_ncep_wigos

@BenjaminRuston thanks, I fixed it. Please check ctest again.

PraveenKumar-NOAA commented 9 months ago

Praveen, thanks for getting around to this long icebox issue that I wrote. Yup, you went the route I think was best to construct a new string out of a series of 4 grouped mixed items. Once you get the ctests passing, I will approve as well.

@gthompsnJCSDA thanks. I updated CMakeLists.txt file, I hope ctest will pass now.

PatNichols commented 9 months ago

@PraveenKumar-NOAA This PR is failing ctests. Looks like a mixup in test files names.

PraveenKumar-NOAA commented 9 months ago

@BenjaminRuston, @PatNichols I don't know what is happening, but ctest is passing for me on Hera.

(gdasapp) -bash-4.2$ ctest -R test_iodaconv_bufr_ncep_wigos Test project /scratch1/NCEPDEV/da/Praveen.Kumar/JEDI/ioda-bundle_wigos_bufr_praveen_new/build Start 639: test_iodaconv_bufr_ncep_wigos 1/1 Test #639: test_iodaconv_bufr_ncep_wigos .... Passed 0.56 sec

100% tests passed, 0 tests failed out of 1

Label Time Summary: iodaconv = 0.56 secproc (1 test) script = 0.56 secproc (1 test)

Total Test time (real) = 0.69 sec (gdasapp) -bash-4.2$

BenjaminRuston commented 9 months ago

@PraveenKumar-NOAA and @emilyhcliu do you mind if we just add this wigos test to the existing ctest that uses the input file rtma_ru.t0000z.adpsfc_nc000101.tm00.bufr_d

@gthompsnJCSDA will push to this branch, we can revert if needed

PraveenKumar-NOAA commented 9 months ago

@BenjaminRuston yes, I think this is fine. I am using an already existing input bufr file from the develop branch for the ctest.

gthompsnJCSDA commented 9 months ago

@PraveenKumar-NOAA What Ben and I decided is that it is simpler/nicer just to include your new WIGOS id directly into the existing adpsfc_rtma test that already uses the existing input file and outputs just one more variable instead of adding the burden of more ctests that do only one additional item. So I modified your branch and I need to wait to build because the output file from the other ctest will also get modified by the changes. So I will push that change in the next 20-30 minutes to finalize everything. This is to help consolidate everything. Please stay tuned.

gthompsnJCSDA commented 9 months ago

@BenjaminRuston @PraveenKumar-NOAA All tests are now passing. To all persons contributing codes, let's consider the idea that not every single unit addition to codes needs its own ctests. Please try to consolidate when the same input files are used in particular.