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.

Sign up for GitHub

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 the "state of ground" to bring in the extra snow depth reports #1368

Draft
wants to merge 20 commits into
base: develop
Choose a base branch
from
Draft

Add the "state of ground" to bring in the extra snow depth reports #1368

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
1c8096f
Add state of ground to increase the snow depth reports.
jiaruidong2017 Nov 9, 2024
365675d
Update the defined index with zero snow depth.
jiaruidong2017 Nov 13, 2024
aee4dc2
Merge branch 'develop' into feature/snow-sogr
jiaruidong2017 Nov 13, 2024
0c02522
Move python code to parm/gdas/snow/ directory to aviod extra copy.
jiaruidong2017 Nov 13, 2024
e57ac67
Modif to only replace the missing/filled snod values.
jiaruidong2017 Nov 13, 2024
4376049
Merge branch 'develop' into feature/snow-sogr
jiaruidong2017 Nov 13, 2024
244693c
Address reviewer's comments.
jiaruidong2017 Nov 14, 2024
76a5e94
Fix py norms error.
jiaruidong2017 Nov 14, 2024
2a72e4d
Add the mapping_path indicating the script backend requirements.
jiaruidong2017 Nov 15, 2024
a2d87fd
Merge branch 'develop' into feature/snow-sogr
jiaruidong2017 Nov 15, 2024
fbd1bb9
Add comments
jiaruidong2017 Nov 15, 2024
db46314
Merge branch 'develop' into feature/snow-sogr
jiaruidong2017 Nov 22, 2024
91f160e
Update the missing value for elevation from bufr file.
jiaruidong2017 Nov 22, 2024
dc8d9b8
Merge branch 'develop' into feature/snow-sogr
jiaruidong2017 Dec 20, 2024
d7bf5d7
Merge branch 'develop' into feature/snow-sogr
jiaruidong2017 Dec 24, 2024
3c1dbf7
Update the scripts and sorc/bufr-query.
jiaruidong2017 Dec 24, 2024
55b54cd
Merge branch 'develop' into feature/snow-sogr
jiaruidong2017 Jan 13, 2025
5cd21ab
Remove the negative snow depth values.
jiaruidong2017 Jan 18, 2025
47bfb19
Merge branch 'develop' into feature/snow-sogr
jiaruidong2017 Jan 18, 2025
bb85eb6
Fix a logic error
jiaruidong2017 Jan 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions parm/snow/obs/config/bufr_sfcsno_mapping.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ bufr:
query: "[*/CLON, */CLONH]"
stationIdentification:
query: "*/RPID"

stationElevation:
query: "[*/SELV, */HSMSL]"

Expand All @@ -26,11 +25,8 @@ bufr:
query: "[*/SNWSQ1/TOSD, */MTRMSC/TOSD, */STGDSNDM/TOSD]"
transforms:
- scale: 1000.0
Comment on lines 27 to 28
Copy link
Collaborator

Choose a reason for hiding this comment

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

Based on IODA data convention, the unit of totalSnowDepth is m. We should remove the conversion of it from meter to millimeter.

Copy link
Collaborator Author

@jiaruidong2017 jiaruidong2017 Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Same as above to keep use the conversion.

filters:
- bounding:
variable: totalSnowDepth
lowerBound: 0
upperBound: 10000000
Copy link
Collaborator

Choose a reason for hiding this comment

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

For my understanding:

  • is this introducing a 10 m limit on snow depth obs?
  • if they are above 10 m, are they discarded, or set to 10.?

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 previously put this large number (10000m) here for removing the missing values. Here, we don't need this any more because we need to set the missing snod values to 0 when sogr satisfies the defined conditions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We might still have missing values though wouldn't we?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The missing values will be removed after applying sogr conditions.

groundState:
query: "[*/GRDSQ1/SOGR, */STGDSNDM/SOGR]"

encoder:
variables:
Expand Down Expand Up @@ -65,11 +61,18 @@ encoder:
coordinates: "longitude latitude"
source: variables/stationIdentification
longName: "Identification of Observing Location"
units: "m"
units: "index"
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jiaruidong2017 The unit of stationIdentification is not defined (empty) in the IODA convention table, and it is left for users to decide. Have you discussed the unit for stationIdentification with JCSDA (Ben Ruston)?
Let's leave it as "index" for now and I will open an issue in IODA repository and check if we can use "index" as unit for this variable.


# ObsValue
- name: "ObsValue/totalSnowDepth"
coordinates: "longitude latitude"
source: variables/totalSnowDepth
longName: "Total Snow Depth"
units: "mm"
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jiaruidong2017 The unit of the totalSnowDepth defined in IODA data convention is m, not mm

https://jointcenterforsatellitedataassimilation-jedi-docs.readthedocs-hosted.com/en/latest/inside/conventions/tbls.html

Copy link
Collaborator Author

@jiaruidong2017 jiaruidong2017 Jan 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

We have to keep using the mm here in the observation files to match the unit used in the UFS model for the snow depth.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jiaruidong2017 Is it possible to read in the totalSnowDepth in m unit from the IODA/NetCDF file and then convert it to mm in the UFS model?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is possible, but will require additional efforts. We can discuss this issue with the physics team.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jiaruidong2017 I guess only a one-line change is necessary for UFS. You just need to convert totalSnowDepth from m to mm where it is read from IODA obs file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This issue (mismatch between units proscribed by JEDI and those used in the UFS) is bigger than this PR - as all other snow depth IODA files are in mm too. If we change these observations to mm, we'll need to change the others. I suggest that we leave this in mm for now, and create a separate issue to address the unit mismatch (by introducing a unit transform somewhere).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed. Let's keep it as "mm" for now.


- name: "ObsValue/groundState"
coordinates: "longitude latitude"
source: variables/groundState
longName: "STATE OF THE GROUND"
units: "index"

9 changes: 6 additions & 3 deletions parm/snow/obs/config/sfcsno_snow.yaml.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@
halo size: 250e3
obsdatain:
engine:
type: bufr
type: script
script file: "{{ USHgfs }}/bufr2ioda_sfcsno_bufr_encoder.py"
args:
input_path: '{{ DATA }}/obs/{{ OPREFIX }}sfcsno.tm00.bufr_d'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion:
Make the mapping file as input to the Python script for create_obs_group

args:
 input_path:  '{{ DATA }}/obs/{{ OPREFIX }}sfcsno.tm00.bufr_d'
 mapping_path: = "./obs/bufr_sfcsno_mapping.yaml"

This will make it clear that the script backend requires two inputs: bufr file and the mapping file

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 for the suggestions. I made the changes as suggested.

mapping_path: '{{ DATA }}/obs/bufr_sfcsno_mapping.yaml'
obsfile: '{{ DATA }}/obs/{{ OPREFIX }}sfcsno.tm00.bufr_d'
mapping file: '{{ DATA }}/obs/bufr_sfcsno_mapping.yaml'
obsdataout:
engine:
type: H5File
Expand Down Expand Up @@ -54,7 +57,7 @@
where:
- variable:
name: MetaData/stationElevation
minvalue: -999.0
maxvalue: 999999999.9
- filter: Domain Check # land only
where:
- variable:
Expand Down
2 changes: 1 addition & 1 deletion sorc/bufr-query
Open in desktop
Submodule bufr-query updated 36 files
+4 −2 CMakeLists.txt
+5 −5 cmake/Targets.cmake
+4 −2 core/CMakeLists.txt
+8 −0 core/include/bufr/BufrParser.h
+25 −2 core/include/bufr/DataContainer.h
+523 −1 core/include/bufr/DataObject.h
+5 −7 core/include/bufr/DataProvider.h
+8 −3 core/include/bufr/File.h
+3 −0 core/include/bufr/NcepDataProvider.h
+3 −0 core/include/bufr/WmoDataProvider.h
+66 −1 core/src/bufr/BufrReader/BufrParser.cpp
+51 −11 core/src/bufr/BufrReader/Query/DataProvider/DataProvider.cpp
+8 −0 core/src/bufr/BufrReader/Query/DataProvider/NcepDataProvider.cpp
+7 −0 core/src/bufr/BufrReader/Query/DataProvider/WmoDataProvider.cpp
+12 −5 core/src/bufr/BufrReader/Query/File.cpp
+56 −0 core/src/bufr/DataContainer.cpp
+11 −2 core/src/encoders/netcdf/Encoder.cpp
+31 −0 docs/python_api.rst
+1 −0 docs/uml/BUFR_BigPicture.puml
+25 −28 python/CMakeLists.txt
+17 −10 python/DataObjectFunctions.cpp
+4 −4 python/DataObjectFunctions.h
+4 −0 python/py_bufr.cpp
+17 −1 python/py_data_container.cpp
+2 −1 python/py_file.cpp
+49 −0 python/py_mpi.cpp
+23 −0 python/py_mpi.h
+15 −2 python/py_parser.cpp
+12 −8 test/CMakeLists.txt
+2 −2 test/testinput/bufrtest_fieldname_validation.py
+88 −0 test/testinput/bufrtest_python_mpi_test.py
+21 −12 test/testinput/bufrtest_python_test.py
+3 −0 tools/bufr2netcdf/CMakeLists.txt
+140 −44 tools/bufr2netcdf/bufr2netcdf.cpp
+1 −1 tools/build_scripts/bufr_query_cpp_lint.py
+11 −3 tools/show_queries/QueryPrinter.cpp
44 changes: 44 additions & 0 deletions ush/ioda/bufr2ioda/bufr2ioda_sfcsno_bufr_encoder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import numpy as np
import bufr
from pyioda.ioda.Engines.Bufr import Encoder


def mask_container(container, mask):
new_container = bufr.DataContainer()
for var_name in container.list():
var = container.get(var_name)
paths = container.get_paths(var_name)
new_container.add(var_name, var[mask], paths)

return new_container


def create_obs_group(input_path, mapping_path):
"""Create the ioda snow observations
This method:
- reads state of ground (sogr) and snow depth (snod)
- applys sogr conditions to the missing snod values
- removes the filled/missing snow values and creates the masked container
- encoders the new container.

Parameters
----------
input_path: The input bufr file
mapping_path: The input bufr2ioda mapping file

"""

YAML_PATH = mapping_path
container = bufr.Parser(input_path, YAML_PATH).parse()

sogr = container.get('variables/groundState')
snod = container.get('variables/totalSnowDepth')
snod[(sogr <= 11.0) & snod.mask] = 0.0
snod[(sogr == 15.0) & snod.mask] = 0.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was your intention to remove these values (treat them as missing) or just to set them to 0 as you have done here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you want to update the mask you should do something like this (instead of zeroing things out):

snod.mask = (sogr <= 11.0) & snod.mask
snod.mask = (sogr == 15.0) & snod.mask

Copy link
Collaborator Author

@jiaruidong2017 jiaruidong2017 Jan 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

We want to set the snod to 0 here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, just FYI I don't think this code does exactly what you think it does. The result of the container get returns a numpy masked array. snod.mask == True values indicate those values that should be masked out (invalid/missing values).

I think the following might be more correct...

snod[(sogr <= 11.0) & (~snod.mask)] = 0.0

I don't think you really need to manually mask out the masked values like this (since you are removing them later anyways). You could probably just do this:

snod[(sogr <= 11.0) | (sogr == 15.0)] = 0.0

Copy link
Collaborator Author

@jiaruidong2017 jiaruidong2017 Jan 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks @rmclaren
We just want to change the snod to zero for the missing snod values. If the snod has the valid values, we will ignore the sogr conditions and keep to use the snod values (probably is 0 or some other numbers).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok

container.replace('variables/totalSnowDepth', snod)

masked_container = mask_container(container, (~snod.mask))

data = next(iter(Encoder(YAML_PATH).encode(masked_container).values()))

return data
Loading