From 1f3c8e73e4137c0bcba8c62813083f7caecd0316 Mon Sep 17 00:00:00 2001 From: Cameron Beccario Date: Wed, 23 Sep 2026 12:47:03 +0900 Subject: [PATCH] grib2: introduce decodeScaledValue utility method and use it wherever scaled values are read from bytes --- .../ucar/nc2/iosp/grib/TestGribSpheroids.java | 2 +- .../main/java/ucar/nc2/grib/GribNumbers.java | 14 ++- .../java/ucar/nc2/grib/grib2/Grib2Gds.java | 15 ++-- .../java/ucar/nc2/grib/grib2/Grib2Pds.java | 44 +++------- .../java/ucar/nc2/grib/TestGribNumbers.java | 88 ++++++++++++++++--- 5 files changed, 108 insertions(+), 55 deletions(-) diff --git a/cdm-test/src/test/java/ucar/nc2/iosp/grib/TestGribSpheroids.java b/cdm-test/src/test/java/ucar/nc2/iosp/grib/TestGribSpheroids.java index 2fcb5d01db..4a5aa3ea68 100644 --- a/cdm-test/src/test/java/ucar/nc2/iosp/grib/TestGribSpheroids.java +++ b/cdm-test/src/test/java/ucar/nc2/iosp/grib/TestGribSpheroids.java @@ -107,7 +107,7 @@ public void code7_oblate_specified_m() throws IOException { try (NetcdfFile ncfile = NetcdfFiles.open(filename, null)) { Variable v = ncfile.findVariable("LambertConformal_Projection"); Attribute axis = v.findAttribute("semi_major_axis"); - Assert.assertEquals(6377397., axis.getNumericValue().doubleValue(), 0.1); + Assert.assertEquals(6377397.16, axis.getNumericValue().doubleValue(), 0.1); } } } diff --git a/grib/src/main/java/ucar/nc2/grib/GribNumbers.java b/grib/src/main/java/ucar/nc2/grib/GribNumbers.java index ac1ee18d51..a8c0360c5e 100644 --- a/grib/src/main/java/ucar/nc2/grib/GribNumbers.java +++ b/grib/src/main/java/ucar/nc2/grib/GribNumbers.java @@ -268,7 +268,7 @@ public static long int8(RandomAccessFile raf) throws IOException { } /** - * A signed byte has a sign bit then 1 15-bit value. + * A signed byte has a sign bit and a 7-bit value. * This is not twos complement (!) * * @param v convert byte to signed int @@ -280,8 +280,16 @@ public static int convertSignedByte(byte v) { return sign * value; } - public static int convertSignedByte2(byte v) { - return (v >= 0) ? (int) v : -(128 + v); + /** + * A scaled value is encoded with the formula: L * 10^F = V, where L is the original value, F is the scale factor and + * V is the scaled value. Therefore, to decode: L = V / 10^F. + * See: https://codes.ecmwf.int/grib/format/grib2/regulations/ + */ + public static double decodeScaledValue(int scaledValue, int scaleFactor) { + // Using p = 10^|F| ensures p is exactly representable. The result is then `L = V / p` or `L = V * p` based on the + // sign of F. This reduces float point rounding error. + double p = Math.pow(10, Math.abs(scaleFactor)); + return scaleFactor <= 0 ? (scaledValue * p) : (scaledValue / p); } // count number of bits on in bitmap diff --git a/grib/src/main/java/ucar/nc2/grib/grib2/Grib2Gds.java b/grib/src/main/java/ucar/nc2/grib/grib2/Grib2Gds.java index a8ad3e2ae7..77d26b073f 100644 --- a/grib/src/main/java/ucar/nc2/grib/grib2/Grib2Gds.java +++ b/grib/src/main/java/ucar/nc2/grib/grib2/Grib2Gds.java @@ -22,6 +22,8 @@ import java.util.Formatter; import java.util.UUID; +import static ucar.nc2.grib.GribNumbers.decodeScaledValue; + /** * Template-specific fields for Grib2SectionGridDefinition * LOOK hashCode not right, cant use approximate float compare @@ -100,7 +102,7 @@ public static Grib2Gds factory(int template, byte[] data) { public int numberOfDataPoints; public int center; - public float earthRadius, majorAxis, minorAxis; // in meters + public double earthRadius, majorAxis, minorAxis; // in meters protected int scanMode; public int earthShape; @@ -296,13 +298,10 @@ int getOctet4(int start) { return GribNumbers.int4(getOctet(start), getOctet(start + 1), getOctet(start + 2), getOctet(start + 3)); } - private float getScaledValue(int start) { - int scaleFactor = getOctetSigned(start); - int scaleValue = getOctet4(start + 1); - if (scaleFactor != 0) - return (float) (scaleValue / Math.pow(10, scaleFactor)); - else - return (float) scaleValue; + private double getScaledValue(int index) { + int scaleFactor = getOctetSigned(index); + int scaledValue = getOctet4(index + 1); + return decodeScaledValue(scaledValue, scaleFactor); } /* diff --git a/grib/src/main/java/ucar/nc2/grib/grib2/Grib2Pds.java b/grib/src/main/java/ucar/nc2/grib/grib2/Grib2Pds.java index 890b2e90a1..8a795fc9b6 100644 --- a/grib/src/main/java/ucar/nc2/grib/grib2/Grib2Pds.java +++ b/grib/src/main/java/ucar/nc2/grib/grib2/Grib2Pds.java @@ -17,6 +17,7 @@ import java.util.StringJoiner; import java.util.zip.CRC32; +import static ucar.nc2.grib.GribNumbers.decodeScaledValue; import static ucar.nc2.grib.grib2.Grib2Utils.intervalToRangeDescriptor; /** @@ -318,11 +319,10 @@ public final int getRawLength() { return input.length; } - - protected double getScaledValue(int start) { - int scale = getOctetSigned(start++); - int value = GribNumbers.int4(getOctet(start++), getOctet(start++), getOctet(start++), getOctet(start++)); - return applyScaleFactor(scale, value); + public final double getScaledValue(int index) { + int scaleFactor = getOctetSigned(index); + int scaledValue = getInt4StartingAtOctet(index + 1); + return decodeScaledValue(scaledValue, scaleFactor); } public int getStatisticalProcessType() { @@ -1019,18 +1019,13 @@ public int getProbabilityType() { } public double getProbabilityLowerLimit() { - int scale = getOctetSigned(38); - int value = GribNumbers.int4(getOctet(39), getOctet(40), getOctet(41), getOctet(42)); - return applyScaleFactor(scale, value); + return getScaledValue(38); } public double getProbabilityUpperLimit() { - int scale = getOctetSigned(43); - int value = GribNumbers.int4(getOctet(44), getOctet(45), getOctet(46), getOctet(47)); - return applyScaleFactor(scale, value); + return getScaledValue(43); } - @Override public int getProbabilityHashcode() { if (probHash == 0) { @@ -1113,7 +1108,7 @@ public String getProbabilityName() { f.format("below_%s", Format.dfrac(getProbabilityUpperLimit(), scale2)); break; default: - f.format("UknownProbType=%d", getProbabilityType()); + f.format("UnknownProbType%d", getProbabilityType()); } result = StringUtil2.removeFromEnd(f.toString(), '0'); } @@ -1479,9 +1474,7 @@ public SatelliteBand[] getSatelliteBands() { sb.series = GribNumbers.int2(getOctet(pos), getOctet(pos + 1)); sb.number = GribNumbers.int2(getOctet(pos + 2), getOctet(pos + 3)); sb.instrumentType = getOctet(pos + 4); - int scaleFactor = getOctetSigned(pos + 5); - int svalue = GribNumbers.int4(getOctet(pos + 6), getOctet(pos + 7), getOctet(pos + 8), getOctet(pos + 9)); - sb.value = applyScaleFactor(scaleFactor, svalue); + sb.value = getScaledValue(pos + 5); pos += 10; result[i] = sb; } @@ -1557,9 +1550,7 @@ public SatelliteBand[] getSatelliteBands() { sb.series = GribNumbers.int2(getOctet(pos), getOctet(pos + 1)); sb.number = GribNumbers.int2(getOctet(pos + 2), getOctet(pos + 3)); sb.instrumentType = GribNumbers.int2(getOctet(pos + 4), getOctet(pos + 5)); - int scaleFactor = getOctetSigned(pos + 6); - int svalue = GribNumbers.int4(getOctet(pos + 7), getOctet(pos + 8), getOctet(pos + 9), getOctet(pos + 10)); - sb.value = applyScaleFactor(scaleFactor, svalue); + sb.value = getScaledValue(pos + 6); pos += octetsPerBand; result[i] = sb; } @@ -1634,9 +1625,7 @@ public SatelliteBand[] getSatelliteBands() { sb.series = GribNumbers.int2(getOctet(pos), getOctet(pos + 1)); sb.number = GribNumbers.int2(getOctet(pos + 2), getOctet(pos + 3)); sb.instrumentType = GribNumbers.int2(getOctet(pos + 4), getOctet(pos + 5)); - int scaleFactor = getOctetSigned(pos + 6); - int svalue = GribNumbers.int4(getOctet(pos + 7), getOctet(pos + 8), getOctet(pos + 9), getOctet(pos + 10)); - sb.value = applyScaleFactor(scaleFactor, svalue); + sb.value = getScaledValue(pos + 6); pos += octetsPerBand; result[i] = sb; } @@ -2191,17 +2180,6 @@ protected CalendarDate calcTime(int startIndex) { return CalendarDate.of(null, year, month, day, hour, minute, second); } - /** - * Apply scale factor to value, return a double result. - * - * @param scale signed scale factor - * @param value apply to this value - * @return value ^ -scale - */ - double applyScaleFactor(int scale, int value) { - return ((scale == 0) || (scale == 255) || (value == 0)) ? value : value * Math.pow(10, -scale); - } - TimeInterval[] readTimeIntervals(int n, int startIndex) { TimeInterval[] result = new TimeInterval[n]; for (int i = 0; i < n; i++) { diff --git a/grib/src/test/java/ucar/nc2/grib/TestGribNumbers.java b/grib/src/test/java/ucar/nc2/grib/TestGribNumbers.java index 20679fcf53..5968bb0d48 100644 --- a/grib/src/test/java/ucar/nc2/grib/TestGribNumbers.java +++ b/grib/src/test/java/ucar/nc2/grib/TestGribNumbers.java @@ -1,25 +1,49 @@ package ucar.nc2.grib; -import static org.junit.Assert.*; -import static ucar.nc2.grib.GribNumbers.convertSignedByte; -import static ucar.nc2.grib.GribNumbers.convertSignedByte2; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import ucar.ma2.DataType; +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertNotEquals; +import static ucar.nc2.grib.GribNumbers.*; + @RunWith(JUnit4.class) public class TestGribNumbers { @Test public void testConvertSignedByte() { - System.out.printf("byte == convertSignedByte == convertSignedByte2 == hex%n"); - for (int i = 125; i < 256; i++) { - byte b = (byte) i; - System.out.printf("%d == %d == %d == %s%n", b, convertSignedByte(b), convertSignedByte2(b), - Long.toHexString((long) i)); - assertEquals(convertSignedByte(b), convertSignedByte2(b)); - } + assertThat(convertSignedByte((byte) 0x00)).isEqualTo(0); + assertThat(convertSignedByte((byte) 0x01)).isEqualTo(1); + assertThat(convertSignedByte((byte) 0x02)).isEqualTo(2); + assertThat(convertSignedByte((byte) 0x7d)).isEqualTo(125); + assertThat(convertSignedByte((byte) 0x7e)).isEqualTo(126); + assertThat(convertSignedByte((byte) 0x7f)).isEqualTo(127); + + assertThat(convertSignedByte((byte) 0x80)).isEqualTo(-0); + assertThat(convertSignedByte((byte) 0x81)).isEqualTo(-1); + assertThat(convertSignedByte((byte) 0x82)).isEqualTo(-2); + assertThat(convertSignedByte((byte) 0xfd)).isEqualTo(-125); + assertThat(convertSignedByte((byte) 0xfe)).isEqualTo(-126); + assertThat(convertSignedByte((byte) 0xff)).isEqualTo(-127); + } + + @Test + public void testConvertSignedInt() { + assertThat(int4(0x00, 0x00, 0x00, 0x00)).isEqualTo(0); + assertThat(int4(0x00, 0x00, 0x00, 0x01)).isEqualTo(1); + assertThat(int4(0x00, 0x00, 0x00, 0x02)).isEqualTo(2); + assertThat(int4(0x7f, 0xff, 0xff, 0xfd)).isEqualTo(2147483645); + assertThat(int4(0x7f, 0xff, 0xff, 0xfe)).isEqualTo(2147483646); + assertThat(int4(0x7f, 0xff, 0xff, 0xff)).isEqualTo(2147483647); + + assertThat(int4(0x80, 0x00, 0x00, 0x00)).isEqualTo(-0); + assertThat(int4(0x80, 0x00, 0x00, 0x01)).isEqualTo(-1); + assertThat(int4(0x80, 0x00, 0x00, 0x02)).isEqualTo(-2); + assertThat(int4(0xff, 0xff, 0xff, 0xfd)).isEqualTo(-2147483645); + assertThat(int4(0xff, 0xff, 0xff, 0xfe)).isEqualTo(-2147483646); + assertThat(int4(0xff, 0xff, 0xff, 0xff)).isEqualTo(UNDEFINED); } @Test @@ -29,4 +53,48 @@ public void testConvertUnsigned() { assertNotEquals(val, val2); } + static void assertDecodes(int scaledValue, int scaleFactor, double expected) { + assertThat(decodeScaledValue(scaledValue, scaleFactor)).isWithin(Math.ulp(expected)).of(expected); + } + + @Test + public void testDecodeScaledValue() { + + assertDecodes(9, 1, 0.9); + assertDecodes(9, 0, 9); + assertDecodes(9, -1, 90); + + assertDecodes(9, 3, 9e-3); + assertDecodes(13, 3, 13e-3); + assertDecodes(18, 3, 18e-3); + assertDecodes(25, 3, 25e-3); + assertDecodes(26, 3, 26e-3); + assertDecodes(36, 3, 36e-3); + assertDecodes(3, 4, 3e-4); + assertDecodes(6, 4, 6e-4); + assertDecodes(9, 4, 9e-4); + assertDecodes(1, 5, 1e-5); + assertDecodes(2, 5, 2e-5); + assertDecodes(10, 5, 10e-5); + assertDecodes(1, -5, 1e5); + assertDecodes(8, -5, 8e5); + assertDecodes(25, -5, 25e5); + assertDecodes(1, -6, 1e6); + assertDecodes(8, -6, 8e6); + assertDecodes(25, -6, 25e6); + assertDecodes(1, -9, 1e9); + assertDecodes(33, -9, 33e9); + assertDecodes(66, -9, 66e9); + + assertDecodes(1, -100, 1e100); + assertDecodes(1, 100, 1e-100); + + assertDecodes(0x7fffffff, 0, 0x7fffffff); + assertDecodes(-1, 0, -1); + + assertThat(decodeScaledValue(15, -127)).isEqualTo(1.5e+128); + assertThat(decodeScaledValue(UNDEFINED, 1)).isEqualTo(-999.9); + assertThat(decodeScaledValue(UNDEFINED, -127)).isEqualTo(-9.999e+130); + + } }