sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Martin Desruisseaux (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (SIS-434) Comparison of MathTransforms should use ContextualParameter
Date Wed, 17 Oct 2018 11:00:00 GMT

     [ https://issues.apache.org/jira/browse/SIS-434?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Martin Desruisseaux updated SIS-434:
------------------------------------
    Description: 
Use case (assuming SIS-433 is not yet fixed): Let _A_ = {{CRS.forCode("EPSG::29871)}} and
_B_ = _A_ formatted in WKT and parsed back. Compare those two CRS. Because of the slight difference
in _"British chain (Sears 1922)"_ unit definition described in SIS-433, we get slightly different
_"Easting at projection centre"_ parameter value after their conversion to metres (parameter
value before conversion is 29352.4763 chSe):

{noformat}
A easting = 590476.8714630401 metres
B easting = 590476.8714630402 metres
{noformat}

Apache SIS implementation splits map projections in _normalization_ → _projection on normalized
ellipsoid_ → _denormalization_ steps. The denormalization matrices are:

{noformat}
A:
┌                                                              ┐
│  3813171.507859623  5084228.678431943  -0.003883839275807552 │
│ -5084228.678431943  3813171.507859623  -0.003303801804941493 │
│        0                  0             1                    │
└                                                              ┘
B:
┌                                                              ┐
│  3813171.507859623  5084228.678431943  -0.003883839175807552 │
│ -5084228.678431943  3813171.507859623  -0.003303801804941493 │
│        0                  0             1                    │
└                                                              ┘
{noformat}

Those matrices are almost identical. They differ only in their _m02_ terms, copied below.
The easting parameter is also shown for comparison purpose (reminder: this parameter differs
in the last digit between the two CRS):

{noformat}
A:            -0.003883839275807552
B:            -0.003883839175807552
Easting = 590476.8714630401 metres
{noformat}

The problem is that while the difference has its origin in the last digit of a map projection
parameter, its effect in the denormalization matrix appears in the middle of a term. The relative
error (A-B)/A is about 2.6E-8, which well above the 1E-13 tolerance threshold currently used
for matrix comparisons. We can not just relax the tolerance threshold since whether the difference
is relevant or not depends on the operations performed before to add this term.

For fixing this issue, comparison of {{MathTransform}} instances should use the information
provided in {{ContextualParameters}} when those information are available. It would allow
to compare the original easting parameters (with a difference only in the last digit) instead
than the transformed value in denormalization matrix.


h2. Testing and debugging
In {{core/sis-referencing/src/test/java/org/apache/sis/test/integration/ConsistencyTest.java}},
put a {{\@Test}} annotation on the {{debug()}} method and temporarily disable other tests.
Put a breakpoint on the following line of code around line 273:

{code:java}
assertTrue(urn, Utilities.deepEquals(crs, parsed, ComparisonMode.DEBUG));
{code}

After execution reaches this point, jump to {{ConcatenatedTransform.equals(Object, ComparisonMode)}}.
This method will be invoked indirectly by {{AbstractCoordinateOperation}}. Proposed fix is
to add the following step: if {{deepEquals(getSteps(), …}} returns {{false}} and {{ComparisonMode.isApproximative()}}
returns {{true}}, then compare again using the pseudo-steps. This may require verifying the
{{equals}} implementation of temporary objects created in those pseudo-steps.

  was:
Use case (assuming SIS-433 is not yet fixed): Let _A_ = {{CRS.forCode("EPSG::29871)}} and
_B_ = _A_ formatted in WKT and parsed back. Compare those two CRS. Because of the slight difference
in _"British chain (Sears 1922)"_ unit definition described in SIS-433, we get slightly different
_"Easting at projection centre"_ parameter value after their conversion to metres (parameter
value before conversion is 29352.4763 chSe):

{noformat}
A easting = 590476.8714630401 metres
B easting = 590476.8714630402 metres
{noformat}

Apache SIS implementation splits map projections in _normalization_ → _projection on normalized
ellipsoid_ → _denormalization_ steps. The denormalization matrices are:

{noformat}
A:
┌                                                              ┐
│  3813171.507859623  5084228.678431943  -0.003883839275807552 │
│ -5084228.678431943  3813171.507859623  -0.003303801804941493 │
│        0                  0             1                    │
└                                                              ┘
B:
┌                                                              ┐
│  3813171.507859623  5084228.678431943  -0.003883839175807552 │
│ -5084228.678431943  3813171.507859623  -0.003303801804941493 │
│        0                  0             1                    │
└                                                              ┘
{noformat}

Those matrices are almost identical. They differ only in their _m02_ terms, copied below.
The easting parameter is also shown for comparison purpose (reminder: this parameter differs
in the last digit between the two CRS):

{noformat}
A:            -0.003883839275807552
B:            -0.003883839175807552
Easting = 590476.8714630401 metres
{noformat}

The problem is that while the difference has its origin in the last digit of a map projection
parameter, its effect in the denormalization matrix appears in the middle of a term. The relative
error (A-B)/A is about 2.6E-8, which well above the 1E-13 tolerance threshold currently used
for matrix comparisons. We can not just relax the tolerance threshold since whether the difference
is relevant or not depends on the operations performed before to add this term.

For fixing this issue, comparison of {{MathTransform}} instances should use the information
provided in {{ContextualParameters}} when those information are available. It would allow
to compare the original easting parameters (with a difference only in the last digit) instead
than the transformed value in denormalization matrix.


> Comparison of MathTransforms should use ContextualParameter
> -----------------------------------------------------------
>
>                 Key: SIS-434
>                 URL: https://issues.apache.org/jira/browse/SIS-434
>             Project: Spatial Information Systems
>          Issue Type: Task
>          Components: Referencing
>            Reporter: Martin Desruisseaux
>            Priority: Major
>
> Use case (assuming SIS-433 is not yet fixed): Let _A_ = {{CRS.forCode("EPSG::29871)}}
and _B_ = _A_ formatted in WKT and parsed back. Compare those two CRS. Because of the slight
difference in _"British chain (Sears 1922)"_ unit definition described in SIS-433, we get
slightly different _"Easting at projection centre"_ parameter value after their conversion
to metres (parameter value before conversion is 29352.4763 chSe):
> {noformat}
> A easting = 590476.8714630401 metres
> B easting = 590476.8714630402 metres
> {noformat}
> Apache SIS implementation splits map projections in _normalization_ → _projection on
normalized ellipsoid_ → _denormalization_ steps. The denormalization matrices are:
> {noformat}
> A:
> ┌                                                              ┐
> │  3813171.507859623  5084228.678431943  -0.003883839275807552 │
> │ -5084228.678431943  3813171.507859623  -0.003303801804941493 │
> │        0                  0             1                    │
> └                                                              ┘
> B:
> ┌                                                              ┐
> │  3813171.507859623  5084228.678431943  -0.003883839175807552 │
> │ -5084228.678431943  3813171.507859623  -0.003303801804941493 │
> │        0                  0             1                    │
> └                                                              ┘
> {noformat}
> Those matrices are almost identical. They differ only in their _m02_ terms, copied below.
The easting parameter is also shown for comparison purpose (reminder: this parameter differs
in the last digit between the two CRS):
> {noformat}
> A:            -0.003883839275807552
> B:            -0.003883839175807552
> Easting = 590476.8714630401 metres
> {noformat}
> The problem is that while the difference has its origin in the last digit of a map projection
parameter, its effect in the denormalization matrix appears in the middle of a term. The relative
error (A-B)/A is about 2.6E-8, which well above the 1E-13 tolerance threshold currently used
for matrix comparisons. We can not just relax the tolerance threshold since whether the difference
is relevant or not depends on the operations performed before to add this term.
> For fixing this issue, comparison of {{MathTransform}} instances should use the information
provided in {{ContextualParameters}} when those information are available. It would allow
to compare the original easting parameters (with a difference only in the last digit) instead
than the transformed value in denormalization matrix.
> h2. Testing and debugging
> In {{core/sis-referencing/src/test/java/org/apache/sis/test/integration/ConsistencyTest.java}},
put a {{\@Test}} annotation on the {{debug()}} method and temporarily disable other tests.
Put a breakpoint on the following line of code around line 273:
> {code:java}
> assertTrue(urn, Utilities.deepEquals(crs, parsed, ComparisonMode.DEBUG));
> {code}
> After execution reaches this point, jump to {{ConcatenatedTransform.equals(Object, ComparisonMode)}}.
This method will be invoked indirectly by {{AbstractCoordinateOperation}}. Proposed fix is
to add the following step: if {{deepEquals(getSteps(), …}} returns {{false}} and {{ComparisonMode.isApproximative()}}
returns {{true}}, then compare again using the pseudo-steps. This may require verifying the
{{equals}} implementation of temporary objects created in those pseudo-steps.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message