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

add ctest for ground-based gnss ipw product #1539

Merged
merged 14 commits into from
Sep 9, 2024

Conversation

BenjaminRuston
Copy link
Collaborator

@BenjaminRuston BenjaminRuston commented Aug 6, 2024

add a ctest for the ground-based gnss integrated precipitable water (IPW) product

Description

add a ctest for an NCEP BUFR file for IPW from ground-based GNSS

Issue(s) addressed

Resolves #<issue_number>

Impact

Potential to add IPW product to assimilation

Checklist

  • I have performed a self-review of my own code
  • I have run the unit tests before creating the PR

@BenjaminRuston BenjaminRuston added the OBS OBS processing, UFO label Aug 6, 2024
@BenjaminRuston BenjaminRuston added the do not merge Something is wrong, do not merge label Aug 6, 2024
@BenjaminRuston
Copy link
Collaborator Author

@ibanos90 added this to see if it properly passes regression testing

the input file, and resulting output file are too large.. @gthompsnJCSDA may be able to help edit the input BUFR to trim it down to well under a MegaByte

for this file at current resolution got a IODArange.py output of:

file: 20240806T00Z_ipw_gnssgb_ncep.nc
range(/MetaData/dateTime): [2024-08-05 23:59:00, 2024-08-06 00:02:00]
range(/MetaData/heightOfStation): [90.0, 90.0]
range(/MetaData/latitude): [-77.83836364746094, 79.98895263671875]
range(/MetaData/longitude): [-170.72244262695312, 179.19654846191406]
range(/MetaData/stationName): [0001-JMA, ZZON-ROBH]
range(/ObsValue/precipitableWater): [2, 72]

@@ -0,0 +1,82 @@
# (C) Copyright 2022 NOAA/NWS/NCEP/EMC
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# (C) Copyright 2022 NOAA/NWS/NCEP/EMC
# (C) Copyright 2024 NOAA/NWS/NCEP/EMC


- name: "MetaData/stationName"
source: variables/stationName
longName: "Station Name"
Copy link
Contributor

@emilyhcliu emilyhcliu Aug 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see "stationName" in the IODA data conventions. How about changing "stationName" to "stationIdentification"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @emilyhcliu appreciate the extra set of eyes.
and as Emily notes there the online document and this too:
https://github.com/JCSDA-internal/ioda/blob/develop/share/ioda/yaml/validation/ObsSpace.yaml

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @emilyhcliu and @BenjaminRuston for letting me help with this. Please proceed with changing"stationName" to "stationIdentification".

One additional change I think didn't get included -

for "heightOfStation", the query value should be "/SELV" instead of "/GNSSRPSQ/ELEV".

@BenjaminRuston BenjaminRuston marked this pull request as draft August 7, 2024 21:00
@BenjaminRuston
Copy link
Collaborator Author

@ibanos90 is the variable that should be a float but is reported as an integer the Observation value itself:
range(/ObsValue/precipitableWater): [2, 72]

will pick this up and try to drag across the finish line

@ibanos90
Copy link
Contributor

ibanos90 commented Sep 4, 2024

@ibanos90 is the variable that should be a float but is reported as an integer the Observation value itself: range(/ObsValue/precipitableWater): [2, 72]

will pick this up and try to drag across the finish line

Yes, that's correct, but @mrixlam can provide more info

@mrixlam
Copy link

mrixlam commented Sep 4, 2024

@ibanos90 is the variable that should be a float but is reported as an integer the Observation value itself: range(/ObsValue/precipitableWater): [2, 72]
will pick this up and try to drag across the finish line

Yes, that's correct, but @mrixlam can provide more info

The variable we discussed about is /ObsValue/precipitableWater.

I have some decoded bufr messages here: /glade/derecho/scratch/mrislam/work/singv_ng_mpas/data/gpsipw in case you have time to take a look at them. Decoded bufr messages have floating point values for precipitableWater but in the converted NetCDF file, it becomes integer and we have no idea why.


globals:
- name: "platformCommonName"
type: string
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrixlam ,,, hearing from @gthompsnJCSDA that adding a line like the but type: float to the precipitableWater should force float

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @BenjaminRuston we previously tried "type: float" for the variables section but it didnt work. But you are right. When I removed "type: float" from the variables section to the exports section, it works. The converted file now have float values for precipitableWater.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused @mrixlam I just tried this with and w/o the type: float

when I added it, I got floats it worked correctly:
range(/ObsValue/precipitableWater): [2.299999952316284, 72.0999984741211]

when removed it output the integer values:
range(/ObsValue/precipitableWater): [2, 72]

so going to add it in... please confirm

query: "*/STSN"

heightOfStation:
query: "*/GNSSRPSQ/ELEV"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

query for heightOfStation should be changed to "*/SELV"

query: "*/STSN"

heightOfStation:
query: "*/GNSSRPSQ/ELEV"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
query: "*/GNSSRPSQ/ELEV"
query: "*/GNSSRPSQ/SELV"

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or this?

Suggested change
query: "*/GNSSRPSQ/ELEV"
query: "*/SELV"

- name: "ObsValue/precipitableWater"
source: variables/precipitableWater
longName: "GPS IPW"
units: "KG/(METER**2)"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrixlam this is consistent with the way units are presented in our verification yaml
https://github.com/JCSDA-internal/ioda/blob/ad84060522f6fa7ab9c75d227f1d9e72c8bf7a28/share/ioda/yaml/validation/ObsSpace.yaml#L570

Suggested change
units: "KG/(METER**2)"
units: "kg m-2"

@BenjaminRuston BenjaminRuston dismissed emilyhcliu’s stale review September 6, 2024 20:46

i've updated to use stationIdentification thanks Emily

@BenjaminRuston BenjaminRuston removed do not merge Something is wrong, do not merge draft labels Sep 7, 2024
@BenjaminRuston BenjaminRuston marked this pull request as ready for review September 7, 2024 01:52
@BenjaminRuston
Copy link
Collaborator Author

@mrixlam and @ibanos90 think I've wrapped this one up,, I'm getting floats for the output and I've reduced the sample file size down to just a few observations to make the ctest dataset much smaller

please let me know if this works for you.. thx

Copy link

@mrixlam mrixlam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current version looks good to me.

Copy link
Contributor

@ibanos90 ibanos90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks so much @BenjaminRuston!

@BenjaminRuston BenjaminRuston merged commit e0ce37b into develop Sep 9, 2024
2 checks passed
@BenjaminRuston BenjaminRuston deleted the feature/ipw_gb_gnss_addition branch September 9, 2024 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OBS OBS processing, UFO
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants