grib: fix NullPointerException for legacy NOAA RAP models #1508
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Attempting to load a [legacy] NOAA Rapid Refresh GRIB2 file fails
with a NullPointerException.
java -jar netcdfAll-5.10.0-SNAPSHOT.jar \
rap_130_20170720_0000_000.grb2
This file loads correctly in netcdf-java 4.6, but it has a quirk:
it contains at least one PDS that does not define a time range.
grib_dump -O -p section_4 \
-w section4Length=58,numberOfTimeRange=0 \
rap_130_20170720_0000_000.grb2
6bef2fb (Fix for issue #606, version 5., 2021-02-26) corrects
time calculations for GRIB2 PDS which have incompatible units for
**time intervals** and related time quantities. The above patch
introduces a `TimeIntervalAndUnits` parser for PDS times.
If a PDS does not define *any* time intervals, the previous
version of
Grib2Tables.getForecastTimeInterval(Grib2Record)
returns a duration of zero—i.e., no elapsed time. This is likely
the correct interpretation if no time intervals are present. After
6bef2fb, this method raises a NullPointerException instead.
This is because the inner function
Grib2Tables.getForecastTimeInterval(Grib2Pds.PdsInterval)
returns an invalid `TimeIntervalAndUnits` when it ought to return
a zero duration instead. The calling functions do not treat this
data type as fallible and may use it in conversions that
return null.
If the PDS does not contain any time intervals, return a
"zero hour" duration instead. This restores the previous behavior.
* In legacy and modern RAP models, PDS which *do* define time
intervals define them as zero. The interpretation of the
"quirky" PDS remains consistent with its brethren.
* This quirk is not present in more [recent] RAP model runs, so
this bug is unlikely to affect current RAP data.
[legacy]: https://www.ncei.noaa.gov/oa/prod-model/rapid-refresh/access/historical/analysis/201707/20170720/rap_130_20170720_0000_000.grb2
[recent]: https://www.ncei.noaa.gov/oa/prod-model/rapid-refresh/access/rap-130-13km/analysis/202504/20250401/rap_130_20250401_0000_000.grb2
|
|
Contributor
Author
|
This passes CI on my fork, so hopefully it will pass here as well. Ready for review. |
Member
|
Thank you so much for your contribution @cbs228! I ran our internal integration tests against your changes as well, and everything passes 👍🏻 |
lesserwhirls
approved these changes
Nov 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Changes
Attempting to load a legacy NOAA Rapid Refresh GRIB2 file fails with a
NullPointerException.stack trace
This file loads correctly in netcdf-java 4.6, but it has a quirk: it contains at least one PDS that does not define a time range.
grib_dump output
6bef2fb (Fix for issue #606, version 5., 2021-02-26) corrects time calculations for GRIB2 PDS which have incompatible units for time intervals and related time quantities. The above patch introduces a
TimeIntervalAndUnitsparser for PDS times.If a PDS does not define any time intervals, the previous version of
returns a duration of zero—i.e., no elapsed time. This is likely the correct interpretation if no time intervals are present. After 6bef2fb, this method raises a
NullPointerExceptioninstead.This is because the inner function
returns an invalid
TimeIntervalAndUnitswhen it ought to return a zero duration instead. The calling functions do not treat this data type as fallible and may use it in conversions that return null.If the PDS does not contain any time intervals, return a "zero hour" duration instead. This restores the previous behavior.
In legacy and modern RAP models, PDS which do define time intervals define them as zero. The interpretation of the "quirky" PDS remains consistent with its brethren.
This quirk is not present in more recent RAP model runs, so this bug is unlikely to affect current RAP data.
PR Checklist
until ready for review