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
commit c0944afb6e2cd77194ea71504ce6d74b651f72b7 Author: Martin Desruisseaux <[email protected]> AuthorDate: Fri Apr 12 12:19:25 2019 +0200 Revert commit a685c8a1bf0b412a544713f1ceda9749a995a5d6 (compute linear regression by minimizing errors in grid indices instead than geospatial coordinates). The reason is because the "compute linear regression in reverse way" approach brings more complexity and is not sufficient anyway; we still needs a more sophisticated iterative algorithm in InterpolatedTransform.Inverse. --- .../operation/builder/LinearTransformBuilder.java | 57 ++-------------------- .../operation/builder/LocalizationGridBuilder.java | 1 - .../operation/builder/ProjectedTransformTry.java | 13 ++--- 3 files changed, 7 insertions(+), 64 deletions(-) diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/builder/LinearTransformBuilder.java b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/builder/LinearTransformBuilder.java index eceb47a..2bb4b0d 100644 --- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/builder/LinearTransformBuilder.java +++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/builder/LinearTransformBuilder.java @@ -37,7 +37,6 @@ import org.opengis.geometry.coordinate.Position; import org.opengis.referencing.operation.Matrix; import org.opengis.referencing.operation.MathTransform; import org.opengis.referencing.operation.MathTransformFactory; -import org.opengis.referencing.operation.NoninvertibleTransformException; import org.apache.sis.geometry.GeneralEnvelope; import org.apache.sis.io.TableAppender; import org.apache.sis.math.Line; @@ -181,16 +180,6 @@ public class LinearTransformBuilder extends TransformBuilder { private transient double[] correlations; /** - * Whether to compute <cite>target to source</cite> transform instead than <cite>source to target</cite>. - * This flag can be set before the first invocation of {@link #create(MathTransformFactory)}. Note that - * if this flag is set, then the {@linkplain #correlations} are values for each source dimensions instead - * than target dimensions. - * - * @see #requestReverseCalculation() - */ - private boolean reverseCalculation; - - /** * Creates a temporary builder with all source fields from the given builder and no target arrays. * Calculated fields, namely {@link #correlations} and {@link #transform}, are left uninitialized. * Arrays are copied by references and their content shall not be modified. The new builder should @@ -1220,34 +1209,6 @@ search: for (int j=domain(); --j >= 0;) { } /** - * Requests {@link #fit()} to compute <cite>target to source</cite> transform instead than <cite>source to target</cite>. - * This flag can be set before the first invocation of {@link #create(MathTransformFactory)}. Note that if this flag is - * set, then the {@linkplain #correlation() correlation coefficients} will be values for each source dimensions instead - * than target dimensions. However the transform returned by {@code create(…)} is still the <cite>source to target</cite> - * transform; only the linear regression has been computed in the reverse way. - * - * <div class="section">Why reverse calculation</div> - * In forward transformation, the linear regression algorithm assumes that grid indices are exact and all errors - * are in geospatial coordinates. This is a reasonable assumption if the linear regression is used directly. But - * in {@link LocalizationGridBuilder}, having the smallest errors on geospatial coordinates is not so important, - * because errors are corrected by the residual grids anyway. However it is important than during inverse operation, - * the grid indices estimated by the linear regression is as close as possible to the real grid indices, otherwise - * we get "no convergence" problem. For that reason, {@link LocalizationGridBuilder} needs to minimize errors on - * the grid indices instead than on the geospatial coordinates. - * - * <div class="section">Constraints</div> - * This method is only for {@link LocalizationGridBuilder} purpose. - * Current {@code LinearTransformBuilder} implementation has the following restrictions: - * <ul> - * <li>Source coordinates must be a two-dimensional grid.</li> - * <li>Source and target dimensions must be equal (in order to have a square matrix).</li> - * </ul> - */ - final void requestReverseCalculation() { - reverseCalculation = (getGridDimensions() == 2) && (targets != null && targets.length == 2); - } - - /** * Creates a linear transform approximation from the source positions to the target positions. * This method assumes that source positions are precise and that all uncertainty is in the target positions. * If {@linkplain #addLinearizers linearizers have been specified}, then this method may project all target @@ -1308,7 +1269,7 @@ search: for (int j=domain(); --j >= 0;) { transformedArrays = tmp.targets; bestCorrelation = altCorrelation; bestCorrelations = altCorrelations; - bestTransform = alt.replace(matrix, altTransform, reverseCalculation); + bestTransform = alt.replace(matrix, altTransform); appliedLinearizer = alt; } else { ProjectedTransformTry.recycle(tmp.targets, pool); @@ -1321,13 +1282,8 @@ search: for (int j=domain(); --j >= 0;) { correlations = bestCorrelations; } } - LinearTransform mt = (LinearTransform) nonNull(factory).createAffineTransform(matrix); - if (reverseCalculation) try { - mt = mt.inverse(); - } catch (NoninvertibleTransformException e) { - throw new FactoryException(e); - } - transform = mt; // Set only on success. + // Set only on success. + transform = (LinearTransform) nonNull(factory).createAffineTransform(matrix); } return transform; } @@ -1382,12 +1338,7 @@ search: for (int j=domain(); --j >= 0;) { if (sources != null) { c = plan.fit(vector(sources[0]), vector(sources[1]), vector(targets[j])); } else try { - if (reverseCalculation) { - Vector vz = Vector.createSequence(0, 1, gridSize[j]).repeat(j != 0, gridSize[j ^ 1]); - c = plan.fit(Vector.create(targets[0]), Vector.create(targets[1]), vz); - } else { - c = plan.fit(gridSize[0], gridSize[1], Vector.create(targets[j])); - } + c = plan.fit(gridSize[0], gridSize[1], Vector.create(targets[j])); } catch (IllegalArgumentException e) { // This may happen if the z vector still contain some "NaN" values. throw new InvalidGeodeticParameterException(noData(), e); diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/builder/LocalizationGridBuilder.java b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/builder/LocalizationGridBuilder.java index e115b41..adda0d8 100644 --- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/builder/LocalizationGridBuilder.java +++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/builder/LocalizationGridBuilder.java @@ -575,7 +575,6 @@ public class LocalizationGridBuilder extends TransformBuilder { @Override public MathTransform create(final MathTransformFactory factory) throws FactoryException { if (transform == null) { - linear.requestReverseCalculation(); // Minimize errors in inverse transform. final LinearTransform gridToCoord = linear.create(factory); /* * Make a first check about whether the result of above LinearTransformBuilder.create() call diff --git a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/builder/ProjectedTransformTry.java b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/builder/ProjectedTransformTry.java index ef87a95..0ae88c1 100644 --- a/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/builder/ProjectedTransformTry.java +++ b/core/sis-referencing/src/main/java/org/apache/sis/referencing/operation/builder/ProjectedTransformTry.java @@ -284,10 +284,9 @@ final class ProjectedTransformTry implements Comparable<ProjectedTransformTry> { * * @param transform the original affine transform. This matrix will not be modified. * @param newValues coefficients computed by {@link LinearTransformBuilder} for the dimensions specified at construction time. - * @param reverseCalculation value of {@link LinearTransformBuilder#reverseCalculation}. * @return a copy of the given {@code transform} matrix with new coefficients overwriting the old values. */ - final MatrixSIS replace(MatrixSIS transform, final MatrixSIS newValues, final boolean reverseCalculation) { + final MatrixSIS replace(MatrixSIS transform, final MatrixSIS newValues) { /* * The two matrices shall have the same number of columns because they were computed with * LinearTransformBuilder instances having the same sources. However the two matrices may @@ -300,14 +299,8 @@ final class ProjectedTransformTry implements Comparable<ProjectedTransformTry> { transform = transform.clone(); for (int j=0; j<projToGrid.length; j++) { final int d = projToGrid[j]; - if (reverseCalculation) { - for (int i=transform.getNumRow(); --i >= 0;) { - transform.setNumber(i, d, newValues.getNumber(i, j)); - } - } else { - for (int i=transform.getNumCol(); --i >= 0;) { - transform.setNumber(d, i, newValues.getNumber(j, i)); - } + for (int i=transform.getNumCol(); --i >= 0;) { + transform.setNumber(d, i, newValues.getNumber(j, i)); } } return transform;
