Skip to content

Commit

Permalink
swap order of enhancements
Browse files Browse the repository at this point in the history
  • Loading branch information
Hailey Johnson committed Mar 15, 2024
1 parent 50407ae commit 16b3027
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 79 deletions.
48 changes: 11 additions & 37 deletions cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@
import ucar.ma2.*;
import ucar.nc2.*;
import ucar.nc2.constants.CDM;
import ucar.nc2.constants.DataFormatType;
import ucar.nc2.dataset.NetcdfDataset.Enhance;
import ucar.nc2.filter.*;
import ucar.nc2.internal.dataset.CoordinatesHelper;
import ucar.nc2.iosp.netcdf3.N3iosp;
import ucar.nc2.util.CancelTask;
import javax.annotation.Nullable;
import java.io.IOException;
Expand Down Expand Up @@ -170,8 +168,6 @@ protected VariableDS(VariableDS vds, boolean isCopy) {
this.unsignedConversion = vds.unsignedConversion;
this.scaleOffset = vds.scaleOffset;
this.convertMissing = vds.convertMissing;
this.fillValue = vds.getFillValue();
this.hasFillValue = vds.hasFillValue();

// Add this so that old VariableDS units agrees with new VariableDS units.
String units = vds.getUnitsString();
Expand Down Expand Up @@ -268,14 +264,14 @@ Array convert(Array data, Set<NetcdfDataset.Enhance> enhancements) {
toApply.add(unsignedConversion);
convertedType = unsignedConversion.getOutType();
}
if (enhancements.contains(Enhance.ApplyScaleOffset) && scaleOffset != null) {
toApply.add(scaleOffset);
convertedType = scaleOffset.getScaledOffsetType();
}
if (enhancements.contains(Enhance.ConvertMissing) && convertMissing != null
&& (dataType == DataType.FLOAT || dataType == DataType.DOUBLE)) {
toApply.add(convertMissing);
}
if (enhancements.contains(Enhance.ApplyScaleOffset) && scaleOffset != null) {
toApply.add(scaleOffset);
convertedType = scaleOffset.getScaledOffsetType();
}
if (enhancements.contains(Enhance.ApplyStandardizer) && standardizer != null) {
toApply.add(standardizer);
}
Expand Down Expand Up @@ -560,8 +556,8 @@ public Array getMissingDataArray(int[] shape) {
}

Array array = Array.factoryConstant(getDataType(), shape, storage);
if (hasFillValue) {
array.setObject(0, fillValue);
if (convertMissing.hasFillValue()) {
array.setObject(0, convertMissing.getFillValue());
}
return array;
}
Expand Down Expand Up @@ -690,12 +686,12 @@ public boolean isInvalidData(double val) {

@Override
public boolean hasFillValue() {
return hasFillValue;
return convertMissing != null && convertMissing.hasFillValue();
}

@Override
public double getFillValue() {
return fillValue;
return convertMissing != null ? convertMissing.getFillValue() : Double.NaN;
}

@Override
Expand Down Expand Up @@ -836,9 +832,6 @@ public Array convert(Array in, boolean convertUnsigned, boolean applyScaleOffset
protected String orgName; // in case Variable was renamed, and we need to keep track of the original name
String orgFileTypeId; // the original fileTypeId.

private boolean hasFillValue = false;
private double fillValue = Double.MAX_VALUE;

protected VariableDS(Builder<?> builder, Group parentGroup) {
super(builder, parentGroup);

Expand Down Expand Up @@ -879,6 +872,9 @@ private void createEnhancements() {
this.unsignedConversion = UnsignedConversion.createFromVar(this);
this.dataType = unsignedConversion != null ? unsignedConversion.getOutType() : dataType;
}
if (this.enhanceMode.contains(Enhance.ConvertMissing)) {
this.convertMissing = ConvertMissing.createFromVariable(this);
}
if (this.enhanceMode.contains(Enhance.ApplyScaleOffset) && (dataType.isNumeric() || dataType == DataType.CHAR)) {
if (this.scaleOffset == null) {
this.scaleOffset = ScaleOffset.createFromVariable(this);
Expand All @@ -893,28 +889,6 @@ private void createEnhancements() {
if (normalizerAtt != null && this.enhanceMode.contains(Enhance.ApplyNormalizer) && dataType.isFloatingPoint()) {
this.normalizer = Normalizer.createFromVariable(this);
}

// need fill value info before convertMissing
Attribute fillValueAtt = findAttribute(CDM.FILL_VALUE);
if (fillValueAtt != null && !fillValueAtt.isString()) {
DataType fillType = FilterHelpers.getAttributeDataType(fillValueAtt, getSignedness());
fillValue = applyScaleOffset(convertUnsigned(fillValueAtt.getNumericValue(), fillType).doubleValue());
hasFillValue = true;
} else {
// No _FillValue attribute found. Instead, if file is NetCDF and variable is numeric, use the default fill value.
boolean isNetcdfIosp = DataFormatType.NETCDF.getDescription().equals(orgFileTypeId)
|| DataFormatType.NETCDF4.getDescription().equals(orgFileTypeId);

if (isNetcdfIosp) {
if (dataType.isNumeric()) {
fillValue = applyScaleOffset(N3iosp.getFillValueDefault(dataType));
hasFillValue = true;
}
}
}
if (this.enhanceMode.contains(Enhance.ConvertMissing)) {
this.convertMissing = ConvertMissing.createFromVariable(this);
}
}

public Builder<?> toBuilder() {
Expand Down
65 changes: 27 additions & 38 deletions cdm/core/src/main/java/ucar/nc2/filter/ConvertMissing.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@
import ucar.ma2.IndexIterator;
import ucar.nc2.Attribute;
import ucar.nc2.constants.CDM;
import ucar.nc2.constants.DataFormatType;
import ucar.nc2.dataset.VariableDS;
import ucar.nc2.util.Misc;
import ucar.nc2.iosp.netcdf3.N3iosp;

import java.util.*;

public class ConvertMissing implements Enhancement {

private boolean hasValidMin, hasValidMax;
private double validMin, validMax, fuzzyValidMin, fuzzyValidMax;
private double validMin, validMax;

private boolean hasFillValue;
private double fillValue; // LOOK: making it double not really correct. What about CHAR?
Expand Down Expand Up @@ -60,36 +61,27 @@ public static ConvertMissing createFromVariable(VariableDS var) {
}
}

// check if validData values are stored packed or unpacked
if (hasValidMin || hasValidMax) {
if (FilterHelpers.rank(validType) == FilterHelpers.rank(var.getScaledOffsetType())
&& FilterHelpers.rank(validType) > FilterHelpers.rank(var.getOriginalDataType())) {
// If valid_range is the same type as the wider of scale_factor and add_offset, PLUS
// it is wider than the (packed) data, we know that the valid_range values were stored as unpacked.
// We already assumed that this was the case when we first read the attribute values, so there's
// nothing for us to do here.
} else {
// Otherwise, the valid_range values were stored as packed. So now we must unpack them.
if (hasValidMin) {
validMin = var.applyScaleOffset(validMin);
}
if (hasValidMax) {
validMax = var.applyScaleOffset(validMax);
}
}
// During the scaling process, it is possible that the valid minimum and maximum values have effectively been
// swapped (for example, when the scale value is negative). Go ahead and check to make sure the valid min is
// actually less than the valid max, and if not, fix it. See https://github.com/Unidata/netcdf-java/issues/572.
if (validMin > validMax) {
double tmp = validMin;
validMin = validMax;
validMax = tmp;
}
}

/// fill_value
boolean hasFillValue = var.hasFillValue();
double fillValue = var.getFillValue();
// need fill value info before convertMissing
Attribute fillValueAtt = var.findAttribute(CDM.FILL_VALUE);
if (fillValueAtt != null && !fillValueAtt.isString()) {
DataType fillType = FilterHelpers.getAttributeDataType(fillValueAtt, var.getSignedness());
fillValue = var.convertUnsigned(fillValueAtt.getNumericValue(), fillType).doubleValue();
hasFillValue = true;
} else {
// No _FillValue attribute found. Instead, if file is NetCDF and variable is numeric, use the default fill value.
String ncfileId = var.getNetcdfFile() == null ? null : var.getNetcdfFile().getFileTypeId();
if (DataFormatType.NETCDF.getDescription().equals(ncfileId)
|| DataFormatType.NETCDF4.getDescription().equals(ncfileId)) {
DataType fillType = var.getDataType();
if (fillType.isNumeric()) {
fillValue = var.convertUnsigned(N3iosp.getFillValueDefault(fillType), fillType).doubleValue();
hasFillValue = true;
}
}
}

/// missing_value
double[] missingValue = null;
Expand Down Expand Up @@ -117,7 +109,6 @@ public static ConvertMissing createFromVariable(VariableDS var) {
DataType missingType = FilterHelpers.getAttributeDataType(missingValueAtt, signedness);
for (int i = 0; i < missingValue.length; i++) {
missingValue[i] = var.convertUnsigned(missingValueAtt.getNumericValue(i), missingType).doubleValue();
missingValue[i] = var.applyScaleOffset(missingValue[i]);
}
}
}
Expand All @@ -135,9 +126,7 @@ public ConvertMissing(boolean fillValueIsMissing, boolean invalidDataIsMissing,
this.hasValidMin = hasValidMin;
this.hasValidMax = hasValidMax;
this.validMin = validMin;
this.fuzzyValidMin = validMin - Misc.defaultMaxRelativeDiffFloat;
this.validMax = validMax;
this.fuzzyValidMax = validMax + Misc.defaultMaxRelativeDiffFloat;
this.hasFillValue = hasFillValue;
this.fillValue = fillValue;
this.missingValue = missingValue;
Expand All @@ -152,10 +141,10 @@ public ConvertMissing(boolean fillValueIsMissing, boolean invalidDataIsMissing,
if (fillValueIsMissing && hasFillValue && mv == fillValue) {
continue;
}
if (invalidDataIsMissing && hasValidMin && mv < fuzzyValidMin) {
if (invalidDataIsMissing && hasValidMin && mv < validMin) {
continue;
}
if (invalidDataIsMissing && hasValidMax && mv > fuzzyValidMax) {
if (invalidDataIsMissing && hasValidMax && mv > validMax) {
continue;
}
missing.add(mv);
Expand Down Expand Up @@ -185,10 +174,10 @@ public boolean isInvalidData(double val) {
if (Double.isNaN(val)) {
return true;
}
if (val > fuzzyValidMax) {
if (val > validMax) {
return true;
}
if (val < fuzzyValidMin) {
if (val < validMin) {
return true;
}
return false;
Expand All @@ -199,7 +188,7 @@ public boolean hasFillValue() {
}

public boolean isFillValue(double val) {
return hasFillValue && Misc.nearlyEquals(val, fillValue, Misc.defaultMaxRelativeDiffFloat);
return hasFillValue && val == fillValue;
}

public double getFillValue() {
Expand All @@ -208,7 +197,7 @@ public double getFillValue() {

public boolean isMissingValue(double val) {
for (double aMissingValue : missingValue) {
if (Misc.nearlyEquals(val, aMissingValue, Misc.defaultMaxRelativeDiffFloat)) {
if (val == aMissingValue) {
return true;
}
}
Expand Down
6 changes: 2 additions & 4 deletions cdm/core/src/test/java/ucar/nc2/filter/TestEnhancements.java
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,8 @@ public static void setUp() throws IOException, InvalidRangeException {

// short data with small scale and offsets
Array smallVals = Array.factory(DataType.FLOAT, new int[] {dataLen}, missingData);

builder.addVariable("smallVals", DataType.FLOAT, "dim").addAttribute(new Attribute(CDM.SCALE_FACTOR, 1e-12))
.addAttribute(new Attribute(CDM.ADD_OFFSET, 1e-12)).addAttribute(new Attribute(CDM.FILL_VALUE, 110));

.addAttribute(new Attribute(CDM.ADD_OFFSET, 1e-12)).addAttribute(new Attribute(CDM.FILL_VALUE, FILL_VALUE));

// write data
NetcdfFormatWriter writer = builder.build();
Expand Down Expand Up @@ -166,7 +164,7 @@ public void testCombinedEnhancements() throws IOException {
public void testConvertMissingWithSmallScaleAndOffset() throws IOException {
Variable v = ncd.findVariable("smallVals");
Array data = v.read();
assertThat(Double.isNaN(data.getDouble(2))).isTrue();
assertThat(Double.isNaN(data.getDouble(6))).isTrue();
assertThat(Double.isNaN(data.getDouble(1))).isFalse();
}
}

0 comments on commit 16b3027

Please sign in to comment.