Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tmerc: adjust coefficient from 1574 to 1575 #1857

Closed
wants to merge 2 commits into from

Conversation

don-vip
Copy link
Contributor

@don-vip don-vip commented Apr 15, 2018

See OSGeo/PROJ#174 which refers to the original paper the code is based on.

See OSGeo/PROJ#174 which refers to the original paper the code is based on.
@aaime
Copy link
Member

aaime commented Apr 16, 2018

A test for any bug fix or feature is required (can be skipped for community modules, but this is core) ;-)

@aaime
Copy link
Member

aaime commented Apr 16, 2018

See also: https://github.com/geotools/geotools/blob/master/CONTRIBUTING.md (github links it every time one opens a pull request, but in a way that's easy to miss)

@don-vip
Copy link
Contributor Author

don-vip commented Apr 22, 2018

@aaime thanks for your reply. I tried to implement a test based on the existing CassiniSoldnerTest and EPSG guidance but it fails right away, I don't understand why?

package org.geotools.referencing.operation.projection;

import static org.geotools.referencing.operation.projection.MapProjection.AbstractProvider.*;
import static org.junit.Assert.*;

import org.geotools.referencing.ReferencingFactoryFinder;
import org.junit.Test;
import org.opengis.parameter.ParameterDescriptor;
import org.opengis.parameter.ParameterValue;
import org.opengis.parameter.ParameterValueGroup;
import org.opengis.referencing.operation.MathTransform;
import org.opengis.referencing.operation.MathTransformFactory;

/**
 * Unit tests of {@link TransverseMercator}.
 *
 * @source $URL$
 */
public class TransverseMercatorTest {

    /**
     * Tests the example provided by the EPSG guidance.
     * See "OGP Surveying and Positioning Guidance Note number 7, part 2 – April 2018", pages 53-57
     */
    @Test
    public void testEpsgExample() throws Exception {
        MathTransformFactory mtFactory = ReferencingFactoryFinder.getMathTransformFactory(null);
        final ParameterValueGroup parameters = mtFactory.getDefaultParameters("Transverse_Mercator");

        // build the transformation using the guidance provided values
        parameter(SEMI_MAJOR, parameters).setValue(6377563.396);
        parameter(SEMI_MINOR, parameters).setValue(6356256.910);
        parameter(LATITUDE_OF_ORIGIN, parameters).setValue(dmsToDegree(49, 0, 0));
        parameter(CENTRAL_MERIDIAN, parameters).setValue(-dmsToDegree(2, 0, 0));
        parameter(SCALE_FACTOR, parameters).setValue(0.9996012717);
        parameter(FALSE_EASTING, parameters).setValue(400000.00);
        parameter(FALSE_NORTHING, parameters).setValue(-100000.00);
        MathTransform transform = mtFactory.createParameterizedTransform(parameters);

        // results as provided by the EPSG guidance
        final double[] point = new double[] { dmsToDegree(50, 30, 0), dmsToDegree(0, 30, 0) };
        final double[] expected = new double[] { 577274.99, 69740.50 };

        // check forward transform
        final double[] forward = new double[2];
        transform.transform(point, 0, forward, 0, 1);
        assertEquals(expected[0], forward[0], 1e-1);
        assertEquals(expected[1], forward[1], 1e-1);

        // check inverse transform
        final double[] inverse = new double[2];
        transform.inverse().transform(expected, 0, inverse, 0, 1);
        assertEquals(point[0], inverse[0], 1e-4);
        assertEquals(inverse[1], inverse[1], 1e-4);
    }
    
    /**
     * Extracts the {@link ParameterValue} for a certain {@link ParameterDescriptor}
     */
    ParameterValue<?> parameter(ParameterDescriptor<?> param, ParameterValueGroup group) {
        return group.parameter(param.getName().getCode());
    }
    
    /**
     * Converts a DMS value into degrees
     */
    double dmsToDegree(double degrees, double minutes, double seconds) {
        return degrees + (minutes + seconds / 60) / 60;
    }
}
org.geotools.referencing.operation.projection.ProjectionException: Le résultat de la projection pourrait se trouver à 45 046,126 mètres de la position attendue. Êtes-vous certain que les coordonnées sont dans la région de validité de cette projection cartographique? Le point est situé à 52°05.7'E du méridien central et à 48°29.1'S de la latitude d'origine. La projection est "Transverse_Mercator".
	at org.geotools.referencing.operation.projection.MapProjection.checkReciprocal(MapProjection.java:708)
	at org.geotools.referencing.operation.projection.MapProjection.transform(MapProjection.java:903)
	at org.geotools.referencing.operation.projection.MapProjection.transform(MapProjection.java:938)
	at org.geotools.referencing.operation.projection.TransverseMercatorTest.testEpsgExample(TransverseMercatorTest.java:46)

@aaime
Copy link
Member

aaime commented Apr 23, 2018

@don-vip I'm guessing here, but it seems you are giving lat/lon ordinates, while the machinery expects lon/lat.

@don-vip
Copy link
Contributor Author

don-vip commented Apr 23, 2018

A dumb error indeed, thanks!

@aaime
Copy link
Member

aaime commented Apr 24, 2018

Now that looks good. Is there a Jira ticket we can use for the release notes?

@don-vip
Copy link
Contributor Author

don-vip commented Apr 24, 2018

Now there is one: https://osgeo-org.atlassian.net/browse/GEOT-5994

@aaime
Copy link
Member

aaime commented Apr 25, 2018

Manually merged to add one missing copyright header, squash commits and fix commit message referring to the ticket: ac076c7

@aaime aaime closed this Apr 25, 2018
@don-vip
Copy link
Contributor Author

don-vip commented Apr 26, 2018

@aaime thank you! Sorry for the copyright header, I thought it was not required for unit tests as the other ones don't have one (I copied CassiniSoldnerTest). Are you going to add copyright headers to all files during the mass source code reformat?

@aaime
Copy link
Member

aaime commented Apr 27, 2018

@don-vip I'm not, but feel free to contribute that change if you want :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants