Add IMS stationIdentification to ioda formatted output#34
Conversation
|
Offline tests are done and passed. The IODA-compatible IMS stationIdentification now will be output as a nine-digit integer. Please review when you get a chance. Thank you! @CoryMartin-NOAA, @youlong.xia@noaa.gov Once this PR is approved, I will submit another PR to ioda-bundle to get these edits in their repo. Thanks! |
|
@YoulongXia-NOAA @yuanxue2870 @jiaruidong2017 should it be an integer or a string? |
According to Line 421-422 in https://github.com/NOAA-EMC/land-SCF_proc/pull/1/files, 'stid' is an integer. |
|
Yes, my question is when we write it out in IODA compatible format, should it be a string? I think yes, but need to confirm |
Here it is an integer. But it does not matter as Python script in ioda-converter can easily convert an integer into string. I recalled that station identification should be a string. |
@YoulongXia-NOAA, Modify ioda coveter uing example data IMSscf.20230501.oro_C48.nc in /scratch2/NCEPDEV/land/Youlong.Xia/CycledLandDA/land-IMS_proc/test, from ioda-bundle to find src and land, and modify ims ioda python converter to add read in and write out station identification. For station identification, you can refer GHCN ioda python converter at the same directory. |
|
I would suggest to use string here for stid showing the actual required format in downstream codes. |
|
OK. Thanks all! Will convert to string as required. Convert the PR to draft during edits. |
|
@yuanxue2870, it is string for station identification after I checked ghcn ioda format data. python uses string_value = str(integer_value) to convert. |
|
@yuanxue2870, ioda-bundle seems to stop to use and now it is jedi-bundle. I also need to learn this bundle. See https://jointcenterforsatellitedataassimilation-jedi-docs.readthedocs-hosted.com/en/latest/using/building_and_running/building_jedi.html. @CoryMartin-NOAA, is this what we need to learn when update any ioda converters? |
|
ioda-converters is its own repository, you can build it through several ways including jedi-bundle, GDASApp, or ObsForge |
|
Redo offline tests and the test case is passed without issues. The IODA-compatible IMS stationIdentification now will be output as a nine-digit string. A snippet of an example ncdump of the stationIdentification part is attached for your reference. Please review when you get a chance. Thank you! @CoryMartin-NOAA, @youlong.xia@noaa.gov, @jiaruidong2017 |
| for i in range(len(lons)): | ||
| strstid[i] = str(stid[i]) |
There was a problem hiding this comment.
| for i in range(len(lons)): | |
| strstid[i] = str(stid[i]) | |
| + strstid = np.vectorize(str)(stid) |
There was a problem hiding this comment.
The suggested edit above did not work. ERROR: Unrecognized data type <U9. I understand the loop is the least favorable option here: I have also tried other integer to string conversions before using a loop, e.g., astype(str) to convert integer array directly, but a similar error message is acquired (i.e., ERROR: Unrecognized data type <U21).
There was a problem hiding this comment.
ok got it, loop is fine in this case
Thanks for pointing out this. So once this PR looks good to you all, I will submit a PR to https://github.com/JCSDA-internal/ioda-converters to make sure the new edits will go to G-W, is that correct? @CoryMartin-NOAA @YoulongXia-NOAA Thanks! |
YoulongXia-NOAA
left a comment
YoulongXia-NOAA
left a comment
There was a problem hiding this comment.
look fine with me. I approve this PR
|
Is this PR still relevant? Have the changes to the IMS IODA converter been added to the JEDI repo? |
I think this PR is needed to be merged for the offline workflow. The JCSDA repo of the ioda-coverters have already merged relevant edits in this PR (JCSDA-internal/ioda-converters#1668). Offline workflow has its own standalone ``jedi/ioda/imsfv3_scf2iodaTemp.py`, which requires being updated through this PR. @CoryMartin-NOAA, please feel free to correct me if I am wrong. |
|
does it make more sense to just use the one in ioda-converters? |
|
I have the same question as Cory. The converter included here was temporary while we waited for the one in the JEDI repo to be updated. Are there still any differences in functionality between the two? |
They are exactly identical. No differences at all. |
Ideally yes if we can make edits in the offline workflow repo to link |
|
Thanks @yuanxue2870 . I will close the PR, and link in the JEDI converter in a separate PR that I'm working on. |
Sounds great! Thanks! |
5 participants
Describe your changes
Summarise all code changes included in PR:
Modified imsfv3_scf2iodaTemp.py to output IMS stationIdentification to the ioda formatted output.
List any associated PRs in the submodules.
NOAA-EMC/land-SCF_proc#1 (Note: this is in a different repo) due to conversations in NOAA-PSL/land-SCF_proc#13
Issue ticket number and link
List the git Issue that this PR addresses:
#35
Test output
Is this PR expected to pass the DA_IMS_test (ie., does it change the output)?
Yes.
Does it pass the DA_IMS_test?
Yes.
If changes to the test results are expected, what are these changes? Provide a link to the output directory when running the test:
N/A
Checklist before requesting a review