You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sis.apache.org by de...@apache.org on 2019/01/31 15:51:35 UTC
[sis] branch geoapi-4.0 updated: Fixed various problem in axis
order, especially when computing a localization grid.
This is an automated email from the ASF dual-hosted git repository.
desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git
The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
new aa7e1ec Fixed various problem in axis order, especially when computing a localization grid.
aa7e1ec is described below
commit aa7e1ec9bf7435365f101d2f0f8ea93a962629ce
Author: Martin Desruisseaux <ma...@geomatys.com>
AuthorDate: Thu Jan 31 16:51:10 2019 +0100
Fixed various problem in axis order, especially when computing a localization grid.
---
.../java/org/apache/sis/internal/netcdf/Axis.java | 48 ++++++++--------------
.../org/apache/sis/internal/netcdf/Variable.java | 4 +-
.../sis/internal/netcdf/impl/ChannelDecoder.java | 11 ++---
.../apache/sis/internal/netcdf/impl/GridInfo.java | 16 ++------
.../sis/internal/netcdf/impl/VariableInfo.java | 4 +-
.../sis/internal/netcdf/ucar/DecoderWrapper.java | 2 +-
.../sis/internal/netcdf/ucar/GridWrapper.java | 36 ++++++++++++----
7 files changed, 57 insertions(+), 64 deletions(-)
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Axis.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Axis.java
index 4701251..4485dda 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Axis.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Axis.java
@@ -55,9 +55,6 @@ import ucar.nc2.constants.CF;
* Whether the array length is 1 or 2 depends on whether the wrapped netCDF axis is an instance of
* {@link ucar.nc2.dataset.CoordinateAxis1D} or {@link ucar.nc2.dataset.CoordinateAxis2D} respectively.
*
- * <p>The natural ordering of axes is based on the order in which dimensions are declared for a variable.
- * This is not necessarily the same order than the order in which variables are listed in the netCDF file.</p>
- *
* @author Martin Desruisseaux (Geomatys)
* @version 1.0
*
@@ -66,7 +63,7 @@ import ucar.nc2.constants.CF;
* @since 0.3
* @module
*/
-public final class Axis extends NamedElement implements Comparable<Axis> {
+public final class Axis extends NamedElement {
/**
* The abbreviation, also used as a way to identify the axis type.
* This is a controlled vocabulary: if any abbreviation is changed,
@@ -409,19 +406,6 @@ public final class Axis extends NamedElement implements Comparable<Axis> {
}
/**
- * Compares this axis with the given axis for ordering based on the dimensions declared in a variable.
- * This is used for sorting axes in the reverse order than the dimension order on the variable using axes.
- * This order is not necessarily related to the order in which axis variables are listed in the netCDF file.
- *
- * @param other the other axis to compare to this axis.
- * @return -1 if this axis should be before {@code other}, +1 if it should be after.
- */
- @Override
- public int compareTo(final Axis other) {
- return other.sourceDimensions[0] - sourceDimensions[0]; // Reverse of netCDF order.
- }
-
- /**
* Creates an ISO 19111 axis from the information stored in this netCDF axis.
*
* @param factory the factory to use for creating the coordinate system axis.
@@ -599,36 +583,36 @@ main: switch (getDimension()) {
*/
final LocalizationGridBuilder createLocalizationGrid(final Axis other) throws IOException, DataStoreException {
if (other.getDimension() == 2) {
- final int d1 = sourceDimensions[0];
- final int d2 = sourceDimensions[1];
- final int o1 = other.sourceDimensions[0];
- final int o2 = other.sourceDimensions[1];
- if ((o1 == d1 && o2 == d2) || (o1 == d2 && o2 == d1)) {
+ final int xd = this.sourceDimensions[0];
+ final int yd = this.sourceDimensions[1];
+ final int xo = other.sourceDimensions[0];
+ final int yo = other.sourceDimensions[1];
+ if ((xo == xd && yo == yd) || (xo == yd && yo == xd)) {
/*
* Found two axes for the same set of dimensions, which implies that they have the same
* shape (width and height) unless the two axes ignored a different amount of NaN values.
* Negative width and height means that their actual values overflow the 'int' capacity,
* which we can not process here.
*/
- final int ri = (d1 <= d2) ? 0 : 1; // Take in account that mainDimensionFirst(…) may have reordered values.
- final int ro = (o1 <= o2) ? 0 : 1;
- final int width = getSize(ri );
- final int height = getSize(ri ^ 1);
- if (other.sourceSizes[ro ] != width ||
- other.sourceSizes[ro ^ 1] != height)
+ final int ri = (xd <= yd) ? 0 : 1; // Take in account that mainDimensionFirst(…) may have reordered values.
+ final int ro = (xo <= yo) ? 0 : 1;
+ final int width = getSize(ri ^ 1); // Fastest varying is right-most dimension.
+ final int height = getSize(ri ); // Slowest varying if left-most dimension.
+ if (other.sourceSizes[ro ^ 1] != width ||
+ other.sourceSizes[ro ] != height)
{
coordinates.error(Grid.class, "getGridGeometry", null,
Errors.Keys.MismatchedGridGeometry_2, getName(), other.getName());
} else {
final LocalizationGridBuilder grid = new LocalizationGridBuilder(width, height);
- final Vector v1 = read();
- final Vector v2 = other.read();
+ final Vector vx = this.read();
+ final Vector vy = other.read();
final double[] target = new double[2];
int index = 0;
for (int y=0; y<height; y++) {
for (int x=0; x<width; x++) {
- target[0] = v1.doubleValue(index);
- target[1] = v2.doubleValue(index);
+ target[0] = vx.doubleValue(index);
+ target[1] = vy.doubleValue(index);
grid.setControlPoint(x, y, target);
index++;
}
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Variable.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Variable.java
index 4cb153e..7cada80 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Variable.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/Variable.java
@@ -712,8 +712,8 @@ public abstract class Variable extends NamedElement {
* they get reordered by the {@link Grid#getAxes(Decoder)} method. In the netCDF UCAR API, this method maps directly
* to {@link ucar.nc2.dataset.CoordinateAxis2D#getCoordValue(int, int)}.
*
- * @param j the fastest varying (right-most) index.
- * @param i the slowest varying (left-most) index.
+ * @param j the slowest varying (left-most) index.
+ * @param i the fastest varying (right-most) index.
* @return the coordinate at the given index, or {@link Double#NaN} if it can not be computed.
* @throws IOException if an I/O operation was necessary but failed.
* @throws DataStoreException if a logical error occurred.
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/ChannelDecoder.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/ChannelDecoder.java
index 80c78f7..aebba26 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/ChannelDecoder.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/ChannelDecoder.java
@@ -868,8 +868,8 @@ public final class ChannelDecoder extends Decoder {
if (names == null) {
return false;
}
- for (int i=names.length; --i >= 0;) {
- final VariableInfo axis = findVariable(names[i].toString());
+ for (final CharSequence name : names) {
+ final VariableInfo axis = findVariable(name.toString());
if (axis == null) {
dimensions.clear();
axes.clear();
@@ -942,7 +942,6 @@ nextVar: for (final VariableInfo variable : variables) {
* and we would not need to check for variables having dimension names. However in practice there is
* incomplete attributes, so we check for other dimensions even if the above loop did some work.
*/
- int mixedFlag = axes.isEmpty() ? 0 : 1;
for (int i=variable.dimensions.length; --i >= 0;) { // Reverse of netCDF order.
final Dimension dimension = variable.dimensions[i];
if (usedDimensions.add(dimension)) {
@@ -951,17 +950,15 @@ nextVar: for (final VariableInfo variable : variables) {
continue nextVar;
}
axes.addAll(axis);
- mixedFlag |= 2;
}
}
/*
* Creates the grid geometry using the given domain and range, reusing existing instance if one exists.
* We usually try to preserve axis order as declared in the netCDF file. But if we mixed axes inferred
* from the "coordinates" attribute and axes inferred from variable names matching dimension names, we
- * are better to sort them.
+ * use axes from "coordinates" attribute first followed by other axes.
*/
- GridInfo grid = new GridInfo(variable.dimensions,
- axes.toArray(new VariableInfo[axes.size()]), mixedFlag == 3);
+ GridInfo grid = new GridInfo(variable.dimensions, axes.toArray(new VariableInfo[axes.size()]));
GridInfo existing = shared.putIfAbsent(grid, grid);
if (existing != null) {
grid = existing;
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/GridInfo.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/GridInfo.java
index 617b1ed..dedd304 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/GridInfo.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/GridInfo.java
@@ -93,22 +93,15 @@ final class GridInfo extends Grid {
private final VariableInfo[] range;
/**
- * Whether axes should be sorted instead than relying on the order found in netCDF file.
- */
- private final boolean sortAxes;
-
- /**
* Constructs a new grid geometry information.
* The {@code domain} and {@code range} arrays often have the same length, but not necessarily.
*
- * @param domain describes the input values of the "grid to CRS" conversion, in netCDF order.
- * @param range the output values of the "grid to CRS" conversion, in CRS order as much as possible.
- * @param sortAxes whether axes should be sorted instead than relying on the order found in netCDF file.
+ * @param domain describes the input values of the "grid to CRS" conversion, in netCDF order.
+ * @param range the output values of the "grid to CRS" conversion, in CRS order as much as possible.
*/
- GridInfo(final Dimension[] domain, final VariableInfo[] range, final boolean sortAxes) {
+ GridInfo(final Dimension[] domain, final VariableInfo[] range) {
this.domain = domain;
this.range = range;
- this.sortAxes = sortAxes;
}
/**
@@ -250,9 +243,6 @@ final class GridInfo extends Grid {
axes[targetDim] = new Axis(abbreviation, axis.getAttributeAsString(CF.POSITIVE),
ArraysExt.resize(indices, i), ArraysExt.resize(sizes, i), axis);
}
- if (sortAxes) {
- Arrays.sort(axes);
- }
return axes;
}
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/VariableInfo.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/VariableInfo.java
index 7b5fda6..44d29c1 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/VariableInfo.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/impl/VariableInfo.java
@@ -824,8 +824,8 @@ final class VariableInfo extends Variable implements Comparable<VariableInfo> {
protected double coordinateForAxis(final int j, final int i) throws IOException, DataStoreException {
assert j >= 0 && j < dimensions[0].length : j;
assert i >= 0 && i < dimensions[1].length : i;
- final long n = dimensions[0].length();
- return read().doubleValue(Math.toIntExact(j + n*i));
+ final long n = dimensions[1].length();
+ return read().doubleValue(Math.toIntExact(i + n*j));
}
/**
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/DecoderWrapper.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/DecoderWrapper.java
index a69bb1e..f2b264d 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/DecoderWrapper.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/DecoderWrapper.java
@@ -446,7 +446,7 @@ public final class DecoderWrapper extends Decoder implements CancelTask {
}
geometries = new Grid[(systems != null) ? systems.size() : 0];
for (int i=0; i<geometries.length; i++) {
- geometries[i] = new GridWrapper(systems.get(i), null);
+ geometries[i] = new GridWrapper(systems.get(i));
}
}
return geometries;
diff --git a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/GridWrapper.java b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/GridWrapper.java
index 5eb02fd..585af16 100644
--- a/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/GridWrapper.java
+++ b/storage/sis-netcdf/src/main/java/org/apache/sis/internal/netcdf/ucar/GridWrapper.java
@@ -16,8 +16,10 @@
*/
package org.apache.sis.internal.netcdf.ucar;
-import java.io.IOException;
import java.util.List;
+import java.util.Map;
+import java.util.HashMap;
+import java.io.IOException;
import ucar.nc2.Dimension;
import ucar.nc2.VariableIF;
import ucar.nc2.constants.AxisType;
@@ -63,13 +65,30 @@ final class GridWrapper extends Grid {
private final List<Dimension> domain;
/**
+ * Other {@code GridWrapper} using the same UCAR coordinate systems but with {@link #domain} dimensions in a different order.
+ * We keep previously created {@code GridWrapper} instances in order to keep the cached value in {@link #geometry} field,
+ * since its computation may be costly.
+ */
+ private final Map<List<Dimension>, GridWrapper> reordered;
+
+ /**
* Creates a new grid geometry for the given netCDF coordinate system.
*
* @param cs the netCDF coordinate system.
*/
- GridWrapper(final CoordinateSystem cs, final List<Dimension> dimensions) {
- netcdfCS = cs;
- domain = (dimensions != null) ? dimensions : cs.getDomain();
+ GridWrapper(final CoordinateSystem cs) {
+ netcdfCS = cs;
+ domain = cs.getDomain();
+ reordered = new HashMap<>();
+ }
+
+ /**
+ * Creates a new grid geometry with the same coordinate system than the given parent.
+ */
+ private GridWrapper(final GridWrapper parent, final List<Dimension> dimensions) {
+ netcdfCS = parent.netcdfCS;
+ domain = dimensions;
+ reordered = parent.reordered;
}
/**
@@ -82,9 +101,12 @@ final class GridWrapper extends Grid {
if (domain.equals(source)) {
return this;
}
- if (domain.size() == source.size() && domain.containsAll(source)) {
- return new GridWrapper(netcdfCS, source);
- }
+ return reordered.computeIfAbsent(source, k -> {
+ if (domain.size() == k.size() && domain.containsAll(k)) {
+ return new GridWrapper(this, k);
+ }
+ return null;
+ });
}
return null;
}