From b418be84f8bef54bb2ce8906bda3b94631382fc8 Mon Sep 17 00:00:00 2001 From: Sean Arms <67096+lesserwhirls@users.noreply.github.com> Date: Mon, 28 Sep 2026 13:20:35 -0600 Subject: [PATCH] Fix bug when explicit pre-enhanced datasets are used in NcML Aggregations Ensure enhancements are not applied twice when datasets in NcML aggregations are enhanced before aggregation. --- .../java/ucar/nc2/dataset/NetcdfDataset.java | 3 +- .../java/ucar/nc2/dataset/VariableDS.java | 99 +++++++++++++++-- .../nc2/internal/dataset/DatasetEnhancer.java | 5 +- .../nc2/internal/ncml/AggProxyReader.java | 19 +++- .../internal/ncml/AggregationExisting.java | 3 + .../nc2/internal/ncml/AggregationNew.java | 4 + .../ucar/nc2/internal/ncml/NcmlReader.java | 2 +- .../main/java/ucar/nc2/ncml/Aggregation.java | 15 ++- .../ucar/nc2/ncml/AggregationExisting.java | 3 + .../java/ucar/nc2/ncml/AggregationNew.java | 4 + .../java/ucar/nc2/ncml/AggregationTiled.java | 3 + .../main/java/ucar/nc2/ncml/NcMLReader.java | 6 +- .../test/data/ncml/enhance/agg/scaledAgg1.nc | Bin 0 -> 8482 bytes .../test/data/ncml/enhance/agg/scaledAgg2.nc | Bin 0 -> 8482 bytes .../data/ncml/enhance/aggMemberEnhanced.ncml | 7 ++ .../ncml/enhance/aggNoMemberEnhanced.ncml | 7 ++ .../ucar/nc2/internal/ncml/TestEnhance.java | 101 +++++++++++++++++- .../ucar/nc2/ncml/TestNcmlReadersCompare.java | 7 +- .../java/ucar/nc2/dt/grid/TestGridSubset.java | 8 +- 19 files changed, 270 insertions(+), 26 deletions(-) create mode 100644 cdm/core/src/test/data/ncml/enhance/agg/scaledAgg1.nc create mode 100644 cdm/core/src/test/data/ncml/enhance/agg/scaledAgg2.nc create mode 100644 cdm/core/src/test/data/ncml/enhance/aggMemberEnhanced.ncml create mode 100644 cdm/core/src/test/data/ncml/enhance/aggNoMemberEnhanced.ncml diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDataset.java b/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDataset.java index 0374e73285..9dc8952d52 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDataset.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/NetcdfDataset.java @@ -889,7 +889,7 @@ public Set getEnhanceMode() { return enhanceMode; } - private void addEnhanceModes(Set addEnhanceModes) { + void addEnhanceModes(Set addEnhanceModes) { ImmutableSet.Builder result = new ImmutableSet.Builder<>(); result.addAll(this.enhanceMode); result.addAll(addEnhanceModes); @@ -1064,6 +1064,7 @@ public void empty() { coordAxes = new ArrayList<>(); coordTransforms = new ArrayList<>(); convUsed = null; + this.enhanceMode = Collections.emptySet(); } /** @deprecated do not use */ diff --git a/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java b/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java index 3d8eddced2..d31b02e42d 100644 --- a/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java +++ b/cdm/core/src/main/java/ucar/nc2/dataset/VariableDS.java @@ -7,6 +7,8 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Sets; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import ucar.ma2.*; import ucar.nc2.*; import ucar.nc2.constants.CDM; @@ -35,6 +37,8 @@ */ public class VariableDS extends Variable implements VariableEnhanced, EnhanceScaleMissingUnsigned { + private static final Logger logger = LoggerFactory.getLogger(VariableDS.class); + static final List ENHANCEMENT_PROVIDERS; static { @@ -263,6 +267,53 @@ boolean needConvert() { || enhancements.contains(Enhance.ApplyScaleOffset) || enhancements.contains(Enhance.ConvertMissing); } + /** + * Will this Variable modify the data values it reads, ie apply one of the data affecting enhancements? + *

+ * Unlike {@link #needConvert()} this only considers the enhancements that this Variable applies itself, + * not those already applied by a wrapped Variable. Used by the aggregation proxy readers to decide if + * they must deliver the data as stored, so that an enhancement is not applied twice. + * + * @param v the Variable to test, may be null. + * @return true if v is a VariableDS that converts the data it reads. + */ + public static boolean appliesDataEnhancements(Variable v) { + if (!(v instanceof VariableDS)) { + return false; + } + Set enhancements = ((VariableDS) v).enhanceMode; + return enhancements.contains(Enhance.ConvertEnums) || enhancements.contains(Enhance.ConvertUnsigned) + || enhancements.contains(Enhance.ApplyScaleOffset) || enhancements.contains(Enhance.ConvertMissing) + || enhancements.contains(Enhance.ApplyRuntimeLoadedEnhancements); + } + + /** + * Find the Variable that an aggregation proxy should read from, so that the data affecting enhancements + * are applied exactly once. + *

+ * If the aggregate Variable {@code mainV} will convert the data itself, and the member Variable + * {@code proxyV} has already applied its own enhancements, read the data as stored instead. + * + * @param proxyV the Variable found in the member (proxied) dataset. + * @param mainV the Variable in the aggregation that the data is being read for. + * @return the Variable to actually read from, never null if proxyV is not null. + */ + public static Variable unenhancedProxy(Variable proxyV, Variable mainV) { + if (!(proxyV instanceof VariableDS) || !appliesDataEnhancements(mainV)) { + return proxyV; + } + VariableDS proxyDS = (VariableDS) proxyV; + Variable orgVar = proxyDS.getOriginalVariable(); + if (orgVar == null || !appliesDataEnhancements(proxyDS)) { + return proxyV; + } + if (logger.isDebugEnabled() && !((VariableDS) mainV).enhanceMode.containsAll(proxyDS.enhanceMode)) { + logger.debug("Aggregation member {} applies enhancements {} not applied by the aggregation variable {}", + proxyV.getFullName(), proxyDS.enhanceMode, ((VariableDS) mainV).enhanceMode); + } + return orgVar; + } + Array convert(Array data) { return convert(data, enhanceMode); } @@ -503,14 +554,36 @@ protected Array _read(Section section) throws IOException, InvalidRangeException return convert(result); } + /** + * The data as stored by this VariableDS, ie without applying the enhancements that this Variable would apply. + * This is the array that {@link #_read()} passes to {@link #convert(Array)}. + */ + Array readUnenhanced() throws IOException { + if (hasCachedData()) { + return super._read(); + } + return proxyReader.reallyRead(this, null); + } + + /** Section of {@link #readUnenhanced()}. */ + Array readUnenhanced(Section section) throws IOException, InvalidRangeException { + if ((null == section) || section.computeSize() == getSize()) { + return readUnenhanced(); + } + if (hasCachedData()) { + return super._read(section); + } + return proxyReader.reallyRead(this, section, null); + } + // do not call directly @Override public Array reallyRead(Variable client, CancelTask cancelTask) throws IOException { + if (this.proxyReader != null && this.proxyReader instanceof ucar.nc2.ncml.Aggregation) { + return this.proxyReader.reallyRead(client, cancelTask); + } + if (orgVar == null) { - // possible aggregation - if (this.proxyReader != null && this.proxyReader instanceof ucar.nc2.ncml.Aggregation) { - return this.proxyReader.reallyRead(client, cancelTask); - } return getMissingDataArray(shape); } @@ -527,6 +600,11 @@ public Array reallyRead(Variable client, CancelTask cancelTask) throws IOExcepti if (ucar.nc2.ncml.Aggregation.instanceOfDatasetProxyReader(this.proxyReader)) { return this.proxyReader.reallyRead(client, cancelTask); } + if (orgVar instanceof VariableDS) { + // orgVar has cached data. The cache holds the data as stored, so hand that to the client + // rather than letting orgVar convert it, otherwise enhancements get applied twice. + return ((VariableDS) orgVar).readUnenhanced(); + } } return orgVar.read(); } @@ -539,11 +617,11 @@ public Array reallyRead(Variable client, Section section, CancelTask cancelTask) if ((null == section) || section.computeSize() == getSize()) return reallyRead(client, cancelTask); + if (this.proxyReader != null && this.proxyReader instanceof ucar.nc2.ncml.Aggregation) { + return this.proxyReader.reallyRead(client, section, cancelTask); + } + if (orgVar == null) { - // possible aggregation - if (this.proxyReader != null && this.proxyReader instanceof ucar.nc2.ncml.Aggregation) { - return this.proxyReader.reallyRead(client, section, cancelTask); - } return getMissingDataArray(shape); } @@ -560,6 +638,11 @@ public Array reallyRead(Variable client, Section section, CancelTask cancelTask) if (ucar.nc2.ncml.Aggregation.instanceOfDatasetProxyReader(this.proxyReader)) { return this.proxyReader.reallyRead(client, section, cancelTask); } + if (orgVar instanceof VariableDS) { + // orgVar has cached data. The cache holds the data as stored, so hand that to the client + // rather than letting orgVar convert it, otherwise enhancements get applied twice. + return ((VariableDS) orgVar).readUnenhanced(section); + } } return orgVar.read(section); } diff --git a/cdm/core/src/main/java/ucar/nc2/internal/dataset/DatasetEnhancer.java b/cdm/core/src/main/java/ucar/nc2/internal/dataset/DatasetEnhancer.java index 4df92edc62..4feba7d624 100644 --- a/cdm/core/src/main/java/ucar/nc2/internal/dataset/DatasetEnhancer.java +++ b/cdm/core/src/main/java/ucar/nc2/internal/dataset/DatasetEnhancer.java @@ -175,7 +175,7 @@ private void enhanceStructure(StructureDS.Builder sdb) { * } */ - private void enhanceVariable(VariableDS.Builder vb) { + private void enhanceVariable(VariableDS.Builder vb) { Set varEnhance = EnumSet.copyOf(wantEnhance); // varEnhance will only contain enhancements not already applied to orgVar. @@ -185,6 +185,9 @@ private void enhanceVariable(VariableDS.Builder vb) { } } + varEnhance.removeAll(vb.enhanceMode); + varEnhance.removeAll(dsBuilder.getEnhanceMode()); + // enhance() may have been called previously, with a different enhancement set. // So, we need to reset to default before we process this new set. // if (vb.orgDataType != null) { diff --git a/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggProxyReader.java b/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggProxyReader.java index b2adee5382..ffb3f78b6b 100644 --- a/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggProxyReader.java +++ b/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggProxyReader.java @@ -1,4 +1,8 @@ -/* Copyright Unidata */ +/* + * Copyright (c) 2019-2025 John Caron and University Corporation for Atmospheric Research/Unidata + * See LICENSE.txt for license information. + */ + package ucar.nc2.internal.ncml; import java.io.IOException; @@ -9,6 +13,7 @@ import ucar.nc2.NetcdfFile; import ucar.nc2.ProxyReader; import ucar.nc2.Variable; +import ucar.nc2.dataset.VariableDS; import ucar.nc2.dataset.VariableEnhanced; import ucar.nc2.util.CancelTask; @@ -33,7 +38,7 @@ public Array reallyRead(Variable mainV, CancelTask cancelTask) throws IOExceptio ncfile = dataset.acquireFile(cancelTask); if ((cancelTask != null) && cancelTask.isCancel()) return null; - Variable proxyV = findVariable(ncfile, mainV); + Variable proxyV = readTarget(ncfile, mainV); return proxyV.read(); } finally { dataset.close(ncfile); @@ -46,7 +51,7 @@ public Array reallyRead(Variable mainV, Section section, CancelTask cancelTask) NetcdfFile ncfile = null; try { ncfile = dataset.acquireFile(cancelTask); - Variable proxyV = findVariable(ncfile, mainV); + Variable proxyV = readTarget(ncfile, mainV); if ((cancelTask != null) && cancelTask.isCancel()) return null; return proxyV.read(section); @@ -57,6 +62,14 @@ public Array reallyRead(Variable mainV, Section section, CancelTask cancelTask) } + /** + * Find the Variable to read from the member dataset. If the member dataset was itself enhanced, and the + * aggregation variable will apply the same enhancements again, read the data as stored instead. + */ + private Variable readTarget(NetcdfFile ncfile, Variable mainV) { + return VariableDS.unenhancedProxy(findVariable(ncfile, mainV), mainV); + } + protected Variable findVariable(NetcdfFile ncfile, Variable mainV) { Variable v = ncfile.findVariable(mainV.getFullNameEscaped()); if (v == null) { // might be renamed diff --git a/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggregationExisting.java b/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggregationExisting.java index fa528c229d..72884fbe6f 100644 --- a/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggregationExisting.java +++ b/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggregationExisting.java @@ -117,6 +117,9 @@ protected void buildNetcdfDataset(CancelTask cancelTask) throws IOException { VariableDS.Builder vagg = VariableDS.builder().setName(v.getShortName()).setDataType(v.getDataType()) .setParentGroupBuilder(rootGroup).setDimensionsByName(v.getDimensionsString()); vagg.setProxyReader(this); + if (v instanceof VariableDS) { + vagg.setOriginalVariable(v); + } BuilderHelper.transferAttributes(v, vagg.getAttributeContainer()); rootGroup.replaceVariable(vagg); diff --git a/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggregationNew.java b/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggregationNew.java index 2999db938f..c3fd0e400f 100644 --- a/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggregationNew.java +++ b/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggregationNew.java @@ -115,6 +115,10 @@ protected void buildNetcdfDataset(CancelTask cancelTask) throws IOException { VariableDS.Builder vagg = VariableDS.builder().setName(aggVar.shortName).setDataType(aggVar.dataType) .setParentGroupBuilder(root).setDimensionsByName(dimName + " " + aggVar.makeDimensionsString()); vagg.setProxyReader(this); + if (aggVar instanceof VariableDS.Builder) { + VariableDS.Builder vds = (VariableDS.Builder) aggVar; + vagg.setOriginalVariable(vds.orgVar); + } BuilderHelper.transferAttributes(aggVar.getAttributeContainer(), vagg.getAttributeContainer()); // _CoordinateAxes if it exists must be modified diff --git a/cdm/core/src/main/java/ucar/nc2/internal/ncml/NcmlReader.java b/cdm/core/src/main/java/ucar/nc2/internal/ncml/NcmlReader.java index 4dbfa7e6e5..4f60dd9b54 100644 --- a/cdm/core/src/main/java/ucar/nc2/internal/ncml/NcmlReader.java +++ b/cdm/core/src/main/java/ucar/nc2/internal/ncml/NcmlReader.java @@ -554,7 +554,7 @@ private static void setEnhanceMode(NetcdfDataset.Builder builder, Element netcdf Set mode = parseEnhanceMode(netcdfElem.getAttributeValue("enhance")); if (mode != null) { // cant just set enhance mode - if (DatasetEnhancer.enhanceNeeded(mode, null)) { + if (DatasetEnhancer.enhanceNeeded(mode, builder.getEnhanceMode())) { DatasetEnhancer enhancer = new DatasetEnhancer(builder, mode, cancelTask); enhancer.enhance(); builder.setEnhanceMode(mode); diff --git a/cdm/core/src/main/java/ucar/nc2/ncml/Aggregation.java b/cdm/core/src/main/java/ucar/nc2/ncml/Aggregation.java index 8b6331d717..ec2fac766b 100644 --- a/cdm/core/src/main/java/ucar/nc2/ncml/Aggregation.java +++ b/cdm/core/src/main/java/ucar/nc2/ncml/Aggregation.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 1998-2025 John Caron and University Corporation for Atmospheric Research/Unidata + * Copyright (c) 1998-2026 John Caron and University Corporation for Atmospheric Research/Unidata * See LICENSE.txt for license information. */ @@ -22,6 +22,7 @@ import ucar.nc2.Variable; import ucar.nc2.dataset.DatasetUrl; import ucar.nc2.dataset.NetcdfDataset; +import ucar.nc2.dataset.VariableDS; import ucar.nc2.dataset.VariableEnhanced; import ucar.nc2.units.DateFormatter; import ucar.nc2.util.CancelTask; @@ -823,7 +824,7 @@ public Array reallyRead(Variable mainV, CancelTask cancelTask) throws IOExceptio ncfile = dataset.acquireFile(cancelTask); if ((cancelTask != null) && cancelTask.isCancel()) return null; - Variable proxyV = findVariable(ncfile, mainV); + Variable proxyV = readTarget(ncfile, mainV); return proxyV.read(); } finally { dataset.close(ncfile); @@ -836,7 +837,7 @@ public Array reallyRead(Variable mainV, Section section, CancelTask cancelTask) NetcdfFile ncfile = null; try { ncfile = dataset.acquireFile(cancelTask); - Variable proxyV = findVariable(ncfile, mainV); + Variable proxyV = readTarget(ncfile, mainV); if ((cancelTask != null) && cancelTask.isCancel()) return null; return proxyV.read(section); @@ -847,6 +848,14 @@ public Array reallyRead(Variable mainV, Section section, CancelTask cancelTask) } } + /** + * Find the Variable to read from the member dataset. If the member dataset was itself enhanced, and the + * aggregation variable will apply the same enhancements again, read the data as stored instead. + */ + protected Variable readTarget(NetcdfFile ncfile, Variable mainV) { + return VariableDS.unenhancedProxy(findVariable(ncfile, mainV), mainV); + } + protected Variable findVariable(NetcdfFile ncfile, Variable mainV) { Variable v = ncfile.findVariable(mainV.getFullNameEscaped()); if (v == null) { // might be renamed diff --git a/cdm/core/src/main/java/ucar/nc2/ncml/AggregationExisting.java b/cdm/core/src/main/java/ucar/nc2/ncml/AggregationExisting.java index c638665f6d..cc4311e067 100644 --- a/cdm/core/src/main/java/ucar/nc2/ncml/AggregationExisting.java +++ b/cdm/core/src/main/java/ucar/nc2/ncml/AggregationExisting.java @@ -111,6 +111,9 @@ protected void buildNetcdfDataset(CancelTask cancelTask) throws IOException { VariableDS vagg = new VariableDS(ncDataset, newGroup, null, v.getShortName(), v.getDataType(), v.getDimensionsString(), null, null); vagg.setProxyReader(this); + if (v instanceof VariableDS) { + vagg.setOriginalVariable(v); + } DatasetConstructor.transferVariableAttributes(v, vagg); newGroup.removeVariable(v.getShortName()); diff --git a/cdm/core/src/main/java/ucar/nc2/ncml/AggregationNew.java b/cdm/core/src/main/java/ucar/nc2/ncml/AggregationNew.java index 61e1e6301e..f78cfeb75f 100644 --- a/cdm/core/src/main/java/ucar/nc2/ncml/AggregationNew.java +++ b/cdm/core/src/main/java/ucar/nc2/ncml/AggregationNew.java @@ -106,6 +106,10 @@ protected void buildNetcdfDataset(CancelTask cancelTask) throws IOException { VariableDS vagg = new VariableDS(ncDataset, newGroup, null, aggVar.getShortName(), aggVar.getDataType(), dimName + " " + aggVar.getDimensionsString(), null, null); vagg.setProxyReader(this); + if (aggVar instanceof VariableDS) { + VariableDS vds = (VariableDS) aggVar; + vagg.setOriginalVariable(vds.getOriginalVariable() != null ? vds.getOriginalVariable() : vds); + } DatasetConstructor.transferVariableAttributes(aggVar, vagg); // _CoordinateAxes if it exists must be modified diff --git a/cdm/core/src/main/java/ucar/nc2/ncml/AggregationTiled.java b/cdm/core/src/main/java/ucar/nc2/ncml/AggregationTiled.java index 136474d6d2..3e00a89319 100644 --- a/cdm/core/src/main/java/ucar/nc2/ncml/AggregationTiled.java +++ b/cdm/core/src/main/java/ucar/nc2/ncml/AggregationTiled.java @@ -91,6 +91,9 @@ protected void buildNetcdfDataset(CancelTask cancelTask) throws IOException { VariableDS vagg = new VariableDS(ncDataset, newGroup, null, v.getShortName(), v.getDataType(), v.getDimensionsString(), null, null); // LOOK what about anon dimensions? vagg.setProxyReader(this); // do the reading here + if (v instanceof VariableDS) { + vagg.setOriginalVariable(v); + } DatasetConstructor.transferVariableAttributes(v, vagg); newGroup.removeVariable(v.getShortName()); diff --git a/cdm/core/src/main/java/ucar/nc2/ncml/NcMLReader.java b/cdm/core/src/main/java/ucar/nc2/ncml/NcMLReader.java index 21abce4e87..5fc481a267 100644 --- a/cdm/core/src/main/java/ucar/nc2/ncml/NcMLReader.java +++ b/cdm/core/src/main/java/ucar/nc2/ncml/NcMLReader.java @@ -554,9 +554,9 @@ private void readNetcdf(String ncmlLocation, NetcdfDataset targetDS, NetcdfFile // enhance means do scale/offset and/or add CoordSystems Set mode = NetcdfDataset.parseEnhanceMode(netcdfElem.getAttributeValue("enhance")); - // if (mode == null) - // mode = NetcdfDataset.getEnhanceDefault(); - targetDS.enhance(mode); + if (mode != null) { + targetDS.enhance(mode); + } // optionally add record structure to netcdf-3 String addRecords = netcdfElem.getAttributeValue("addRecords"); diff --git a/cdm/core/src/test/data/ncml/enhance/agg/scaledAgg1.nc b/cdm/core/src/test/data/ncml/enhance/agg/scaledAgg1.nc new file mode 100644 index 0000000000000000000000000000000000000000..efb6e4c9fedc46430c08c2c5271ab187571eec49 GIT binary patch literal 8482 zcmeHMYitx%6h6DtZnxX%11(mN$F%-HUUjK$1yWer-7QqwUFcR&K$q$6v>n{e;>@%Z zsfsab)x<>f2lB%oL=zID)M$kGS_O=W4-$Gx=PiD_W8YobvFN%`uUmZA-C(r|9=nUC zTw40v;KCs_HTXNzUSlm*kgPuN%(9+Kcn!d#rBqEZK6hJVz%!e7+f^vjcB*4a%BU!@ z`)}j@0#%oAF&OXT)EARXsT&k83zF6@SR_Cy)G}%D_JI$LM8HLXegBc`OK&3xCWtgCW=N@WehQqZL?S_WFw0FY`c{NX1W&uDk zk;R2=eTl`fgu<~|svhsx)xjZ?MR`WtREBpdx@?Z<%H6gtwtoE656D(r{V$e`hYX6? z)hl)6xJi9^Zc^8l8cxSee_6N6QmJ<{VR`7hzq$# zy39DL?B&Tg)+?z|RYS{0Te=!Y==<^ZHnEilMiq9jn0JH}=8@2(5-PJ4ZSY(XMj{h?YTbzd|K}H`opf< z?kLwzd}uzE#K)cnW7YKJlnw$80uBNW0uBNW0uBNW0uBNW0uBNW0uBNW0@n%w%s6;3 zX~43I#Ipp`5%fo%vZt=8Ghbnxh}Rxg+9MtbZqU;#vt+95W(^Y`4eX2ggHc7t%am13 zOW6GiyMkNQ+_=o&xNON#a-d~ZQ)5%3f5u-mhYNEc2PHO*k!1a{4C^(`fuKysUpe%< z&HM%`3Nbh)aly9R@`)*cG6whb^FWj|q!M#Hln27}Xki`mHidP}(*!{E6-xB9#2Oiq z!JtH#_o@azW~O?>o#EbaBp8lcnWeijF?yBwC+;?E>Fa7m9k`Esmn;VhueQn!s ziNz0T`rEq~M0ad@^MUip=F~3ZgGc-u%*{t0yr8Tee!@JlXXVC6@B1`fGPqWIe&pnM RsP~!szt~pIdy_-DI^19=nUC zTweOz;KCtwb@)5eUSkbbkgPuN%(9-#cn!d#rBzKaK6hJVz%!e7+f}I0cBtb@+NdhC z`|sfW0iC_Jd%T#@s_lc~YhU?!udR82M&<9`NFd!ANI34!5!ngcvaj_4J0L@t#;T5f64o!XVDWb^*%yglK1P*cy-M z;CNFVQ7o;Z0(3F7kPh(zNU!Bn>y;@4XHH)so4E!rSIBC{22txD zRt<@LWJy!ZV5nP4F%4;kn$e_zQB%?~W}$0L)>U~Rtu#wS?UpW4;nUJ31BKDK*E=e% z#E@J9mD9!W{Qn)pWmktGDh*)KdmD6);amT0hf)bUEb=q($*z;!&)gJr};#OI~M+Rz|zAvzuJ(stV5#MCy!Ymz+ zvMbC5Up>T%%tet66JSBSDxSBNR@|6dD8vzT-jLOBz*ta&0~t}f9cQo>Tmet-axb1x z!a}U)avc9Vr-(AVQ@{N^$nC1$ehQqZLR&7w4;d7( zr%&q2bCdcD+@zinc}NkL<1m56HCEU)Ca2X@LYK86B_A;%`sk6kCP*Uy2UZ;0D=y$3 z=?dehv6m;~Sf`}NR1GZ~ZRuJZq3_4LI>dG!7**N9V!;tom`6gBN~p}`#ARr1xMM3q zOcn*07@RfOg6H2^iM+Ukq2TGsZ$Crqy|=J&Ah@2FumrfB7a&HsWVH-5=IRU4W~3+H z4;ad$gt!QrdMQVblJ2#SHGEENKdY<>#v;}Za55v>4ZSY(XMj{h?fE|ed|LkX`opf< z?kL|*d}uzE#K)cnXsd^?yT?K)PHcxF9VI+ zQ;!}VpBt4Qcx~%%$wiN7`rEta$F^^L^Pvl=*7Q!}gU9^q%}qxizNoAkdCEMwd&P#w TAN({?Hnc{2Vf56*sfm98;y3M$ literal 0 HcmV?d00001 diff --git a/cdm/core/src/test/data/ncml/enhance/aggMemberEnhanced.ncml b/cdm/core/src/test/data/ncml/enhance/aggMemberEnhanced.ncml new file mode 100644 index 0000000000..27cc36038d --- /dev/null +++ b/cdm/core/src/test/data/ncml/enhance/aggMemberEnhanced.ncml @@ -0,0 +1,7 @@ + + + + + + + diff --git a/cdm/core/src/test/data/ncml/enhance/aggNoMemberEnhanced.ncml b/cdm/core/src/test/data/ncml/enhance/aggNoMemberEnhanced.ncml new file mode 100644 index 0000000000..1b6e4a0184 --- /dev/null +++ b/cdm/core/src/test/data/ncml/enhance/aggNoMemberEnhanced.ncml @@ -0,0 +1,7 @@ + + + + + + + diff --git a/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestEnhance.java b/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestEnhance.java index 41585fd5d6..9c75cab4f5 100644 --- a/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestEnhance.java +++ b/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestEnhance.java @@ -1,7 +1,8 @@ /* - * Copyright (c) 1998-2020 John Caron and University Corporation for Atmospheric Research/Unidata + * Copyright (c) 1998-2026 John Caron and University Corporation for Atmospheric Research/Unidata * See LICENSE for license information. */ + package ucar.nc2.internal.ncml; import static com.google.common.truth.Truth.assertThat; @@ -10,9 +11,14 @@ import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import ucar.ma2.Array; import ucar.ma2.DataType; +import ucar.ma2.InvalidRangeException; +import ucar.ma2.MAMath; +import ucar.ma2.MAMath.MinMax; import ucar.nc2.NetcdfFile; import ucar.nc2.Variable; +import ucar.nc2.dataset.NetcdfDataset; import ucar.nc2.dataset.NetcdfDatasets; import ucar.unidata.util.test.TestDir; @@ -21,6 +27,17 @@ public class TestEnhance { private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private static String dataDir = TestDir.cdmLocalTestDataDir + "ncml/enhance/"; + // aggregation of two members whose lat/lon are stored in radians (scale_factor 57.29578) and whose + // ir_brightness_temperature is a packed ushort (scale_factor 0.01) + private static final String aggNoMemberEnhancedLocation = dataDir + "aggNoMemberEnhanced.ncml"; + private static final String aggMemberEnhanceLocation = dataDir + "aggMemberEnhanced.ncml"; + private static final double LAT_MIN = 28.0; + private static final double LAT_MAX = 48.0; + private static final double LON_MIN = -100.0; + private static final double LON_MAX = -79.0; + private static final double DATA_MIN = 194.66; + private static final double DATA_MAX = 319.50; + @Test public void testStandaloneNoEnhance() throws IOException { try (NetcdfFile ncfile = NetcdfDatasets.openFile(dataDir + "testStandaloneNoEnhance.ncml", null)) { @@ -112,4 +129,86 @@ public void testStandaloneDoubleEnhanceDataset() throws IOException { } } + @Test + public void testEnhancedAgg() throws IOException { + try (NetcdfDataset ncd = NetcdfDatasets.openDataset(aggNoMemberEnhancedLocation)) { + testAgg(ncd); + } + + // nested datasets with enhance="true" + try (NetcdfDataset ncd = NetcdfDatasets.openDataset(aggMemberEnhanceLocation)) { + testAgg(ncd); + } + } + + @Test + public void testEnhancedAggOldApi() throws IOException { + try (NetcdfDataset ncd = NetcdfDataset.openDataset(aggNoMemberEnhancedLocation)) { + testAgg(ncd); + } + + // nested datasets with enhance="true" + try (NetcdfDataset ncd = NetcdfDataset.openDataset(aggMemberEnhanceLocation)) { + testAgg(ncd); + } + } + + @Test + public void testUnenhancedAggOfEnhancedDatasets() throws IOException { + // an aggregation that is not itself enhanced must still see the values enhanced by its members + try (NetcdfFile ncfile = NetcdfDatasets.openFile(aggMemberEnhanceLocation, null)) { + checkMemberEnhancedValues(ncfile); + } + try (NetcdfFile ncfile = NetcdfDatasets.openDataset(aggMemberEnhanceLocation, false, null)) { + checkMemberEnhancedValues(ncfile); + } + } + + private void checkMemberEnhancedValues(NetcdfFile ncfile) throws IOException { + Variable lat = ncfile.findVariable("latitude"); + assertThat((Object) lat).isNotNull(); + MinMax latMinMax = MAMath.getMinMax(lat.read()); + assertThat(latMinMax.min).isWithin(1.0e-3).of(LAT_MIN); + assertThat(latMinMax.max).isWithin(1.0e-3).of(LAT_MAX); + } + + private void testAgg(NetcdfDataset ncd) throws IOException { + assertThat(ncd.findDimension("time").getLength()).isEqualTo(2); + + Variable var = ncd.findVariable("ir_brightness_temperature"); + assertThat((Object) var).isNotNull(); + MinMax minMax = MAMath.getMinMax(var.read()); + assertThat(minMax.min).isWithin(0.1).of(DATA_MIN); + assertThat(minMax.max).isWithin(0.1).of(DATA_MAX); + + checkCoord(ncd, "latitude", LAT_MIN, LAT_MAX); + checkCoord(ncd, "longitude", LON_MIN, LON_MAX); + } + + private void checkCoord(NetcdfDataset ncd, String name, double expectedMin, double expectedMax) throws IOException { + Variable coord = ncd.findVariable(name); + assertThat((Object) coord).isNotNull(); + + Array data = coord.read(); + MinMax minMax = MAMath.getMinMax(data); + assertThat(minMax.min).isWithin(1.0e-3).of(expectedMin); + assertThat(minMax.max).isWithin(1.0e-3).of(expectedMax); + + // reading a second time (data may now be cached) must give the same answer + MinMax again = MAMath.getMinMax(coord.read()); + assertThat(again.min).isWithin(1.0e-6).of(minMax.min); + assertThat(again.max).isWithin(1.0e-6).of(minMax.max); + + // a section must agree with the corresponding part of the full read + try { + Array section = coord.read(new int[] {0}, new int[] {2}); + MinMax sectionMinMax = MAMath.getMinMax(section); + assertThat(sectionMinMax.min).isWithin(1.0e-6) + .of(MAMath.getMinMax(data.section(new int[] {0}, new int[] {2})).min); + assertThat(sectionMinMax.max).isWithin(1.0e-6) + .of(MAMath.getMinMax(data.section(new int[] {0}, new int[] {2})).max); + } catch (InvalidRangeException e) { + throw new RuntimeException(e); + } + } } diff --git a/cdm/core/src/test/java/ucar/nc2/ncml/TestNcmlReadersCompare.java b/cdm/core/src/test/java/ucar/nc2/ncml/TestNcmlReadersCompare.java index 890b485ad9..15a5f76fc7 100644 --- a/cdm/core/src/test/java/ucar/nc2/ncml/TestNcmlReadersCompare.java +++ b/cdm/core/src/test/java/ucar/nc2/ncml/TestNcmlReadersCompare.java @@ -1,7 +1,8 @@ /* - * Copyright (c) 1998-2018 University Corporation for Atmospheric Research/Unidata + * Copyright (c) 1998-2026 University Corporation for Atmospheric Research/Unidata * See LICENSE for license information. */ + package ucar.nc2.ncml; import static org.junit.Assert.fail; @@ -84,6 +85,10 @@ public boolean accept(File pathname) { // Bug in old reader if (name.contains("testStandaloneNoEnhance.ncml")) return false; + // Bug in old reader: an aggregation of pre-enhanced datasets that is read without enhancement applies + // the enhancements of the members a second time, see TestEnhance. + if (name.contains("aggMemberEnhanced.ncml")) + return false; if (name.contains("AggFmrc")) return false; // not implemented if (name.endsWith("ml")) diff --git a/cdm/image/src/test/java/ucar/nc2/dt/grid/TestGridSubset.java b/cdm/image/src/test/java/ucar/nc2/dt/grid/TestGridSubset.java index 445050e1eb..8b447eb1db 100644 --- a/cdm/image/src/test/java/ucar/nc2/dt/grid/TestGridSubset.java +++ b/cdm/image/src/test/java/ucar/nc2/dt/grid/TestGridSubset.java @@ -80,7 +80,7 @@ public void testAggByteGiniSubsetStride() throws Exception { assert null != gcs; assert grid.getRank() == 3; int[] org_shape = grid.getShape(); - assert grid.getDataType() == DataType.UINT; + assert grid.getDataType() == DataType.USHORT; Array data_org = grid.readDataSlice(0, 0, -1, -1); assert data_org != null; @@ -88,7 +88,7 @@ public void testAggByteGiniSubsetStride() throws Exception { int[] data_shape = data_org.getShape(); assert org_shape[1] == data_shape[0]; assert org_shape[2] == data_shape[1]; - assert data_org.getElementType() == int.class : data_org.getElementType(); + assert data_org.getElementType() == short.class : data_org.getElementType(); logger.debug("original bbox = {}", gcs.getBoundingBox()); @@ -102,7 +102,7 @@ public void testAggByteGiniSubsetStride() throws Exception { GridCoordSystem gcs2 = grid_section.getCoordinateSystem(); assert null != gcs2; assert grid_section.getRank() == 3; - assert grid_section.getDataType() == DataType.UINT; + assert grid_section.getDataType() == DataType.USHORT; ProjectionRect subset_prect = gcs2.getBoundingBox(); logger.debug("resulting bbox = {}", subset_prect); @@ -112,7 +112,7 @@ public void testAggByteGiniSubsetStride() throws Exception { Array data = grid_section.readVolumeData(1); assert data != null; assert data.getRank() == 2; - assert data.getElementType() == int.class; + assert data.getElementType() == short.class; int[] shape = data.getShape(); assert Math.abs(org_shape[1] - 2 * shape[0]) < 2 : org_shape[2] + " != " + (2 * shape[0]);