sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] 01/03: 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.
Date Fri, 12 Apr 2019 18:22:44 GMT
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 <martin.desruisseaux@geomatys.com>
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;


Mime
View raw message