Merge pull request #1603 from lesserwhirls/valid

Unpacked valid min/max values

Author: lesserwhirls

cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java

@@ -973,6 +973,9 @@ private void createEnhancements() {
       this.unsignedConversion = UnsignedConversion.createFromVar(this);
       this.dataType = unsignedConversion.getOutType();
     }
+    // this needs to be created before the scale/offset enhancement
+    // to properly handle the case where a variable is packed but the valid
+    // max/min values (or range) are not.
     if (this.enhanceMode.contains(Enhance.ConvertMissing)) {
       this.convertMissing = ConvertMissing.createFromVariable(this);
     }

cdm/core/src/main/java/ucar/nc2/filter/ConvertMissing.java

@@ -1,7 +1,13 @@
+/*
+ * Copyright (c) 2022-2026 University Corporation for Atmospheric Research/Unidata
+ * See LICENSE for license information.
+ */
+
 package ucar.nc2.filter;
 
 import ucar.ma2.Array;
 import ucar.ma2.DataType;
+import ucar.ma2.DataType.Signedness;
 import ucar.ma2.IndexIterator;
 import ucar.nc2.Attribute;
 import ucar.nc2.constants.CDM;
@@ -14,6 +20,7 @@
 public class ConvertMissing implements Enhancement {
 
   private boolean hasValidMin, hasValidMax;
+  // If variable is packed and these must be packed values
   private double validMin, validMax;
 
   private boolean hasFillValue;
@@ -35,12 +42,14 @@ public static ConvertMissing createFromVariable(VariableDS var) {
     // assume here its in units of unpacked data. correct this below
     Attribute validRangeAtt = var.findAttribute(CDM.VALID_RANGE);
     DataType validType = null;
+    boolean validRangeDifferentDataType = false;
     if (validRangeAtt != null && !validRangeAtt.isString() && validRangeAtt.getLength() > 1) {
       validType = FilterHelpers.getAttributeDataType(validRangeAtt, signedness);
       validMin = var.convertUnsigned(validRangeAtt.getNumericValue(0), validType).doubleValue();
       validMax = var.convertUnsigned(validRangeAtt.getNumericValue(1), validType).doubleValue();
       hasValidMin = true;
       hasValidMax = true;
+      validRangeDifferentDataType = !validType.equals(var.getDataType());
     }
 
     Attribute validMinAtt = var.findAttribute(CDM.VALID_MIN);
@@ -52,15 +61,27 @@ public static ConvertMissing createFromVariable(VariableDS var) {
         validType = FilterHelpers.getAttributeDataType(validMinAtt, signedness);
         validMin = var.convertUnsigned(validMinAtt.getNumericValue(), validType).doubleValue();
         hasValidMin = true;
+        validRangeDifferentDataType = !validType.equals(var.getDataType());
       }
 
       if (validMaxAtt != null && !validMaxAtt.isString()) {
         validType = FilterHelpers.largestOf(validType, FilterHelpers.getAttributeDataType(validMaxAtt, signedness));
         validMax = var.convertUnsigned(validMaxAtt.getNumericValue(), validType).doubleValue();
         hasValidMax = true;
+        validRangeDifferentDataType = !validType.equals(var.getDataType());
       }
     }
 
+    if (validRangeDifferentDataType && !signedness.equals(Signedness.UNSIGNED)) {
+      // Signal that valid range (or min/max) was specified in unpacked values, so we
+      // need to repack those values. Only applies when the DataTypes do not match because
+      // the variable is unsigned.
+      double scale = var.attributes().findAttributeDouble(CDM.SCALE_FACTOR, 1);
+      double offset = var.attributes().findAttributeDouble(CDM.ADD_OFFSET, 0);
+      validMin = (validMin - offset) / scale;
+      validMax = (validMax - offset) / scale;
+    }
+
     if (validMin > validMax) {
       double temp = validMin;
       validMin = validMax;
@@ -113,6 +134,7 @@ public ConvertMissing(boolean fillValueIsMissing, boolean invalidDataIsMissing,
     this.missingDataIsMissing = missingDataIsMissing;
     this.hasValidMin = hasValidMin;
     this.hasValidMax = hasValidMax;
+    // If variable data is packed, validMin, validMax must also be packed
     this.validMin = validMin;
     this.validMax = validMax;
     this.hasFillValue = hasFillValue;
@@ -150,14 +172,37 @@ public boolean hasValidData() {
     return hasValidMin || hasValidMax;
   }
 
+  /**
+   *
+   * Return the minimum valid value used to enhance a variable.
+   * <p>
+   * If the variable is packed, this value will also be packed.
+   *
+   * @return the minimum valid value as a double.
+   */
   public double getValidMin() {
     return validMin;
   }
 
+  /**
+   *
+   * Return the maximum valid value used to enhance a variable.
+   * <p>
+   * If the variable is packed, this value will also be packed.
+   *
+   * @return the maximum valid value as a double.
+   */
   public double getValidMax() {
     return validMax;
   }
 
+  /**
+   *
+   * Return true if the value is outside the valid range.
+   *
+   * @param val the value to test (must be packed if the variable is packed).
+   * @return true if the value is invalid.
+   */
   public boolean isInvalidData(double val) {
     if (Double.isNaN(val)) {
       return true;

cdm/core/src/test/java/ucar/nc2/dataset/TestScaleOffsetMissing.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1998-2020 University Corporation for Atmospheric Research/Unidata
+ * Copyright (c) 2020-2026 University Corporation for Atmospheric Research/Unidata
  * See LICENSE for license information.
  */
 
@@ -26,6 +26,13 @@ public class TestScaleOffsetMissing {
   private static final byte expectedValidMin = 1;
   private static final byte expectedValidMax = 2;
 
+  // Same thing as above, but for variable that is packed with an unpacked valid_range (min/max)
+  private static final float[] expectedMismatch = new float[] {NaN, 5.0f, 7.0f, 9.0f};
+  private static final int expectedValidMinMismatchUnpacked = 4;
+  private static final int expectedValidMaxMismatchUnpacked = 9;
+  private static final float expectedValidMinMismatchPacked = 1.5f;
+  private static final float expectedValidMaxMismatchPacked = 4;
+
   @Rule
   public TemporaryFolder tempFolder = new TemporaryFolder();
 
@@ -128,4 +135,35 @@ public void testNegScaleOffsetValidRangeDeprecatedApi() throws URISyntaxExceptio
       }
     }
   }
+
+  @Test
+  public void testScaleOffsetValidRangeDiffTypes() throws URISyntaxException, IOException {
+    File testResource = new File(getClass().getResource("testScaleOffsetMissing.ncml").toURI());
+
+    try (NetcdfDataset ncd = NetcdfDatasets.openDataset(testResource.getAbsolutePath(), true, null)) {
+      // Same as scaleOffsetValidMaxMin, but uses valid_range attribute instead of valid_min and valid_max attributes.
+      VariableDS var = (VariableDS) ncd.findVariable("packedUnmatchedType");
+
+      // Packed value of valid min, max should only be used internally to ConvertMissing, so make sure it is
+      // not leaking through
+      assertThat(var.getValidMin()).isNotWithin(fpTol).of(expectedValidMinMismatchPacked);
+      assertThat(var.getValidMax()).isNotWithin(fpTol).of(expectedValidMaxMismatchPacked);
+      // Make sure unpacked values still make it through
+      assertThat(var.getValidMin()).isWithin(fpTol).of(expectedValidMinMismatchUnpacked);
+      assertThat(var.getValidMax()).isWithin(fpTol).of(expectedValidMaxMismatchUnpacked);
+
+      // This will only work if the unpacked values of valid min/max are used by
+      // ConvertMissing
+      float[] actual = (float[]) var.read().getStorage();
+      for (int i = 0; i < actual.length; i++) {
+        if (var.isInvalidData(actual[i])) {
+          assertThat(actual[i]).isNaN();
+          assertThat(expectedMismatch[i]).isNaN();
+        } else {
+          assertThat(actual[i]).isNotNaN();
+          assertThat(actual[i]).isWithin(fpTol).of(expectedMismatch[i]);
+        }
+      }
+    }
+  }
 }

cdm/core/src/test/resources/ucar/nc2/dataset/testScaleOffsetMissing.ncml

@@ -26,4 +26,12 @@
     <attribute name="valid_range" type="byte" value="0 100" />
     <values>-1 0 100 101</values>
   </variable>
+  <variable name="packedUnmatchedType" type="int" shape="4">
+    <attribute name="scale_factor" type="float" value="2.0" />
+    <attribute name="add_offset" type="float" value="1.0" />
+    <attribute name="valid_range" type="float" value="4 9" />
+    <!-- When opening as a NetcdfDataset, the first value should be missing, where as the remaining
+         three values should be valid. -->
+    <values>1 2 3 4</values>
+  </variable>
 </netcdf>