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

grib2: Support for General Unstructured Grid (template 101) #1427

Merged
merged 6 commits into from
Mar 25, 2025

Conversation

michaeldiener
Copy link
Contributor

Description of Changes

Added support for grib2 template 101 for General Unstructured Grid.
As this is unstructured data, there was no fitting projection, so I also added an UnstructuredProjection.

Added a unit test as well. One test is synthetic, so no file needed. There is a 2nd test method (testIconGrib2FileRead) prepared, but the file is 5.9 MB and so this is why I didn't enable it. Please advise on how to handle this.

Last but not least, the motivation for adding template 101 is that the ICON model of the German weather service (DWD) is using this template and otherwise those files cannot be read.

PR Checklist

  • Link to any issues that the PR addresses
  • Add labels
  • Open as a draft PR
    until ready for review
  • Make sure GitHub tests pass
  • Mark PR as "Ready for Review"

adding support for template 101 (General Unstructured Grid)
adding support for template 101 (General Unstructured Grid)
... code style ...
@michaeldiener michaeldiener changed the title Maint 5.x grib2icon grib2: Support for General Unstructured Grid (template 101) Mar 20, 2025
@michaeldiener
Copy link
Contributor Author

Test seems to fail after I updated the branch to the latest version, before it was fine.

According to the log, the underlying reason seems to be:

dap4.test.TestConstraints > initializationError FAILED
2025-03-20T12:53:20.7479283Z     java.lang.ExceptionInInitializerError
2025-03-20T12:53:20.7479921Z         at dap4.test.TestConstraints.<clinit>(TestConstraints.java:58)
2025-03-20T12:53:20.7480348Z 
2025-03-20T12:53:20.7480460Z         Caused by:
2025-03-20T12:53:20.7484856Z         org.testcontainers.containers.ContainerFetchException: Can't get Docker image: RemoteDockerImage(imageName=<resolving>, imagePullPolicy=DefaultPullPolicy(), imageNameSubstitutor=org.testcontainers.utility.ImageNameSubstitutor$LogWrappedImageNameSubstitutor@5bf1e07)
2025-03-20T12:53:20.7488197Z             at org.testcontainers.containers.GenericContainer.getDockerImageName(GenericContainer.java:1364)
2025-03-20T12:53:20.7492007Z             at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:359)
2025-03-20T12:53:20.7493907Z             at org.testcontainers.containers.GenericContainer.start(GenericContainer.java:330)
2025-03-20T12:53:20.7495017Z             at ucar.unidata.util.test.DapTestContainer.<clinit>(DapTestContainer.java:31)
2025-03-20T12:53:20.7514495Z             ... 1 more
2025-03-20T12:53:20.7514775Z 
2025-03-20T12:53:20.7515181Z             Caused by:
2025-03-20T12:53:20.7516538Z             com.github.dockerjava.api.exception.DockerClientException: Could not build image: toomanyrequests: You have reached your unauthenticated pull rate limit. https://www.docker.com/increase-rate-limit
2025-03-20T12:53:20.7518410Z                 at com.github.dockerjava.api.command.BuildImageResultCallback.getImageId(BuildImageResultCallback.java:71)
2025-03-20T12:53:20.7519784Z                 at com.github.dockerjava.api.command.BuildImageResultCallback.awaitImageId(BuildImageResultCallback.java:50)
2025-03-20T12:53:20.7521056Z                 at org.testcontainers.images.builder.ImageFromDockerfile.resolve(ImageFromDockerfile.java:163)
2025-03-20T12:53:20.7522208Z                 at org.testcontainers.images.builder.ImageFromDockerfile.resolve(ImageFromDockerfile.java:42)
2025-03-20T12:53:20.7523243Z                 at org.testcontainers.utility.LazyFuture.getResolvedValue(LazyFuture.java:20)
2025-03-20T12:53:20.7524116Z                 at org.testcontainers.utility.LazyFuture.get(LazyFuture.java:41)
2025-03-20T12:53:20.7525203Z                 at org.testcontainers.shaded.com.google.common.util.concurrent.Futures$1.get(Futures.java:536)
2025-03-20T12:53:20.7526303Z                 at org.testcontainers.images.RemoteDockerImage.getImageName(RemoteDockerImage.java:130)
2025-03-20T12:53:20.7527320Z                 at org.testcontainers.images.RemoteDockerImage.resolve(RemoteDockerImage.java:67)
2025-03-20T12:53:20.7528282Z                 at org.testcontainers.images.RemoteDockerImage.resolve(RemoteDockerImage.java:28)
2025-03-20T12:53:20.7529223Z                 at org.testcontainers.utility.LazyFuture.getResolvedValue(LazyFuture.java:20)
2025-03-20T12:53:20.7530061Z                 at org.testcontainers.utility.LazyFuture.get(LazyFuture.java:41)
2025-03-20T12:53:20.7531030Z                 at org.testcontainers.containers.GenericContainer.getDockerImageName(GenericContainer.java:1362)
2025-03-20T12:53:20.7531834Z                 ... 4 more

@lesserwhirls
Copy link
Collaborator

First off, thank you for your contribution!

Test seems to fail after I updated the branch to the latest version, before it was fine.

It looks like the gradle testcontainers ran into an issue pulling a base image from dockerhub due to too many unauthenticated requests - I will look into getting that fixed.

There is a 2nd test method (testIconGrib2FileRead) prepared, but the file is 5.9 MB and so this is why I didn't enable it. Please advise on how to handle this.

We run nightly integration testing that requires access to ~100 GiB of test data, and that set of extended test data would be a good place to store a file of that size. You can mark the test with ucar.unidata.util.test.category.NeedsCdmUnitTest using @Category(NeedsCdmUnitTest.class), and it will be skipped unless access to the extended test datasets is available. If you upload a copy of the file (assuming there are no distribution restrictions), I can tell you how to modify the test to reference the test file in our extended test datasets collection. TestDiscontiguousInterval.java is a concrete example of what it looks like.

With regards to your contribution, my immediate first thought is concerning UnstructuredProjection. I think what we want is something along the lines of cdm/core/src/main/java/ucar/unidata/geoloc/projection/LatLonProjection.java, which may work as-is. That said, perhaps you have been down that rabbit hole already?

@lesserwhirls lesserwhirls added enhancement New feature or request iosp: grib grib file format grib2 labels Mar 20, 2025
@michaeldiener
Copy link
Contributor Author

file.zip

Thanks!

Regarding the test file, this is the copyright we should probably mention somewhere in a readme where the file is stored: “CC BY 4.0 Source: Deutscher Wetterdienst”
I have uploaded the file here, but had to zip as otherwise it was not accepted as an upload.

Regarding the UnstructuredProjection, I created this because no other ProjectionImpl was really fitting. The grib2 template is “unstructured data” so it is explicitly not based on latitudes and longitudes. Also the UnstructuredProjection contains parameters vital for working with the unstructured data later and then be able to associate it with geo coordinates:

addParameter(EARTH_SHAPE, earthShape);
addParameter(NUMBER_OF_GRID_USED, numberOfGridUsed);
addParameter(NUMBER_OF_GRID_IN_REFERENCE, numberOfGridInReference);
addParameter(UUID, uuid);

If another ProjectionImpl works better, please let me know, I’m open for suggestions.

One more thing, I’ll send another commit right now that will improve reading compressed files by 300%. It’s just a small change and hope we can include it in this PR.

@michaeldiener
Copy link
Contributor Author

PS: Regarding the performance improvement, it is basically using a BufferedReader...

@michaeldiener
Copy link
Contributor Author

I've also prepared the test method for the real file and am waiting for you to let me know the file location. Thanks.

@lesserwhirls
Copy link
Collaborator

Ok, now that I've pulled down your PR locally and played around with the sample GRIB message, I think I better see why you needed to go with UnstructuredProjection. Do octets
20-35 (Universally Unique Identifier of horizontal grid) give enough information to be able to uniquely construct one dimensional lat/lon variables?

In terms of the test dataset, use the following for the location:

"TestDir.cdmUnitTestDir + /formats/grib2/ugrid/icon_global_icosahedral_single-level_2025031912_004_T_2M.grib2"

@michaeldiener
Copy link
Contributor Author

Thank you. I'll push the changes in a minute.

Do octets 20-35 (Universally Unique Identifier of horizontal grid) give enough information to be able to uniquely construct one dimensional lat/lon variables?

Not exactly construct, but it is enough to find the needed information. As far as I understand it, the UUID corresponds to number_of_grid_used and number_of_grid_in_reference and identifies a specific grid. In the example file it points to this grid.

The grid itself consists of icosahedron and looks approximately like this:
image

To map this to actual lat/lon variables one has to load another file and then use the same indices to get lat/lon. In the example link you load the file grids/public/edzw/icon_grid_0026_R03B07_G.nc. To be more specific, the unstructured data is usually one-dimensional and the value at index 5 can be mapped to lat/lon in the grid definition file with the same index 5. As of the example grid structure we have triangles and so this can be translated to a middle point or 3 edge points.

@michaeldiener
Copy link
Contributor Author

As far as I understand, in general there also could be other grid structures than icosahedron for unstructured data.

@lesserwhirls
Copy link
Collaborator

Thank you for the additional information. I think at this point you have it right - this will at least allow users to load the data, and they will need to use more information to make full use of it. For now, I just don't see us shipping a 1.7 GiB netCDF file to be able to work with these particular files in geophysical space. Perhaps at some point in the future we will have a better way of dealing with the actual grid information, but that will take much more thought.

Thank you again for your contribution!

@lesserwhirls lesserwhirls merged commit 854b225 into Unidata:maint-5.x Mar 25, 2025
13 checks passed
@michaeldiener
Copy link
Contributor Author

Thank you! Yes, and that one netCDF file is just for one specific grid specification and there are many others.

@michaeldiener michaeldiener deleted the maint-5.x_grib2icon branch March 26, 2025 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request grib2 iosp: grib grib file format
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants