Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python converter for MSONET BUFR DUMP #192

Merged
merged 4 commits into from
Oct 22, 2024
Merged

Conversation

PraveenKumar-NOAA
Copy link
Contributor

  • This PR adds a Python converter for MSONET BUFR DUMP data.

  • The converter will be used to transform MSONET BUFR data into IODA (netCDF) format. Following steps similar to GDASApp, a Python file and a JSON description file were created.

  • The following two files are new:

  1. ~/IODA/python/bufr2ioda_msonet.py
  2. ~/IODA/python/bufr2ioda_msonet.json

@hu5970
Copy link
Contributor

hu5970 commented Oct 19, 2024

@delippi @SamuelDegelia-NOAA Could you review this PR. This PR should not impact other part of the App. Thanks, Ming

@SamuelDegelia-NOAA
Copy link
Contributor

I think a lot of the comments from #181 will probably be the same here. One thing is that we need the run_bufr2ioda.sh utility to run the converters added here, but that tool is only included in #181 which still hasn't been merged. I will copy it over for testing but just an FYI if anyone else wants to test this PR too.

@PraveenKumar-NOAA do you happen to have any mesonet bufr files to test this with? We do not have a rap.t00z.msonet.tm00.bufr_d file staged for any of the cases in RDASApp, only the larger prepbufr files.

@PraveenKumar-NOAA
Copy link
Contributor Author

I think a lot of the comments from #181 will probably be the same here. One thing is that we need the run_bufr2ioda.sh utility to run the converters added here, but that tool is only included in #181 which still hasn't been merged. I will copy it over for testing but just an FYI if anyone else wants to test this PR too.

@PraveenKumar-NOAA do you happen to have any mesonet bufr files to test this with? We do not have a rap.t00z.msonet.tm00.bufr_d file staged for any of the cases in RDASApp, only the larger prepbufr files.

@SamuelDegelia-NOAA most of the comments on #181 are about naming and unit changes. I followed the JEDI standards here and do not agree with making those changes.

I will send you path for a msonet bufr file through chat, please check this PR. Thanks!

@SamuelDegelia-NOAA
Copy link
Contributor

I think a lot of the comments from #181 will probably be the same here. One thing is that we need the run_bufr2ioda.sh utility to run the converters added here, but that tool is only included in #181 which still hasn't been merged. I will copy it over for testing but just an FYI if anyone else wants to test this PR too.
@PraveenKumar-NOAA do you happen to have any mesonet bufr files to test this with? We do not have a rap.t00z.msonet.tm00.bufr_d file staged for any of the cases in RDASApp, only the larger prepbufr files.

@SamuelDegelia-NOAA most of the comments on #181 are about naming and unit changes. I followed the JEDI standards here and do not agree with making those changes.

I will send you path for a msonet bufr file through chat, please check this PR. Thanks!

@PraveenKumar-NOAA No problem! For any of the changes that you agree with in #181, would you mind making the corresponding changes here (if needed)? For the others, we can keep the discussion inside #181.

@SamuelDegelia-NOAA
Copy link
Contributor

SamuelDegelia-NOAA commented Oct 22, 2024

Thanks @PraveenKumar-NOAA - I was able to run these tools successfully in RDASApp as well after adding copying over the files in #181!

A couple of quick comments from my first test:

  • I noticed that the IODA files created here are missing the ObsError and ObsType groups. We are using ObsType for some filtering in JEDI. Do these data exist in the bufr files and would it be possible to include them in the IODA data?
  • The MetaData/timeOffset variable is missing - we definitely need this one
  • In the prepbufr python converter, metaData/pressure is a float but here it is an int. I am not sure if this is important (@delippi might know)

@hu5970 hu5970 requested a review from spanNOAA October 22, 2024 17:06
@SamuelDegelia-NOAA
Copy link
Contributor

The ObsType and ObsError data do not exist in bufr files, so we cannot expect these IODA files to be the same as the prepbufr converters.

As with #181, I am okay with merging this now to get the framework for the python-based converters into RDASApp. Then we can modify more as we go forward.

@delippi
Copy link
Collaborator

delippi commented Oct 22, 2024

I agree as well.

One thing I want to mention that doesn't hold anything up is since ObsError doesn't exist in bufr then why do we use it in prepbufr? I've been an advocate for not having ObsError in the IODA and just require users to specify the errors to use.

@SamuelDegelia-NOAA
Copy link
Contributor

@PraveenKumar-NOAA @delippi Do we happen to know if anyone (e.g., GDAS or another JEDI application) is using the obs error data from prepbufr? Maybe this inclusion of ObsError in the bufr2ioda converters is a carry over from another application? I know in GSI that regional configurations were forced to read obs errors from errtable and not the prepbufr files. So it makes sense not to include the prepbufr obs errors in RDASApp.

However, I experienced an issue with running GETKF in split mode if ObsError does not exist in the input IODA files. An empty DerivedObsError then got created which led to the solver showing zero increments. So there at least seems to be some expectation in JEDI for ObsError to exist even if it isn't used.

@delippi
Copy link
Collaborator

delippi commented Oct 22, 2024

@SamuelDegelia-NOAA, my guess is the logic is that since it is prepbufr it ought to be in IODA. I don't know how anyone else is using it or if they just specify directly. I assume when you run in split mode, you are unable to specify the ObsError in some part of that application? If that is the case, then I guess that is the only argument needed to keep it in. But then what would we do in the scenario where we're using bufr that doesn't have that info?

@SamuelDegelia-NOAA
Copy link
Contributor

SamuelDegelia-NOAA commented Oct 22, 2024

@delippi I don't have the ObsError-less files anymore to confirm, but the problem was related to the DerivedObsError group. This group was created by default in the hofx files if ObsError did not exist. It was getting created even if we assigned the errors through the yaml. But DerivedObsError was always empty for me for some reason and that led to the zero increments.

There might have been an easy fix for this, I just never really got around to trying much. Thinking about it now, maybe this could have been solved by removing defer to post from the assign error filter.

But yeah, if we don't have that info in the bufr files then this would be an issue to solve (way down the road).

@hu5970 hu5970 self-requested a review October 22, 2024 20:52
@hu5970 hu5970 merged commit dffa109 into develop Oct 22, 2024
3 checks passed
@hu5970 hu5970 deleted the feature/msonet_python branch October 22, 2024 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants