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

Enhancing unitconvert to support arbitrary conversion factor #1053

Closed
rouault opened this Issue Jun 21, 2018 · 15 comments

Comments

Projects
None yet
2 participants
@rouault
Member

rouault commented Jun 21, 2018

Currently, +proj=unitconvert only supports a fixed set of linear units. The EPSG database currently supports 59 length units, whereas pj_unit 21 (and potentially someone could create a WKT with a completely custom unit)
In the old pj_init() interface there was the +to_meter= and +vto_meter= options to be able to specify an arbitrary conversion factor.
Why not reinteroducing this capability in +proj=unitconvert with +xy_in_factor, +xy_out_factor, +z_in_factor and +z_out_factor whose values would be te conversion factor to metre ?

@rouault

This comment has been minimized.

Member

rouault commented Jun 21, 2018

Or just keep existing parameter names, but accept numeric values instead of the unit names

@kbevers

This comment has been minimized.

Member

kbevers commented Jun 21, 2018

+to_meter and +vto_meter works with the new API:

$ echo 12 56 10 | cct +proj=merc
 1335833.8895   7522963.2413       10.0000           inf
$ echo 12 56 10 | cct +proj=merc +to_meter=2.0 +vto_meter=2.0
  667916.9448   3761481.6206        5.0000           inf

unitconvert was really only introduced to handle time units. It was easy to add distance units as well so I did. I have been thinking instead introducing a +tunits parameter for handling time units instead of unitconvert.

@rouault

This comment has been minimized.

Member

rouault commented Jun 21, 2018

OK, I actually tried with +proj=cart and they are ignored

$ echo "90 0 0" | src/cct +proj=pipeline +step +proj=cart +a=1000 +b=1000  +to_meter=1000 +vto_meter=1000
       0.0000      1000.0000        0.0000           inf

Same for +proj=geocent

@rouault

This comment has been minimized.

Member

rouault commented Jun 21, 2018

ok, inv_prepare() and fwd_finalize() do ignore fr_meter/to_meter/etc. in the PJ_IO_UNITS_CARTESIAN case. That should be fixed right ?

@rouault

This comment has been minimized.

Member

rouault commented Jun 21, 2018

Actually for cartesian, we should only take into account +to_meter and apply it to the 3 axis (doesn't make sense to use one unit for XY, and another one for Z)

@kbevers

This comment has been minimized.

Member

kbevers commented Jun 21, 2018

ok, inv_prepare() and fwd_finalize() do ignore fr_meter/to_meter/etc. in the PJ_IO_UNITS_CARTESIAN case. That should be fixed right ?

Not entirely sure it should. Maybe. In what case do you need to change the units of cartesian coordinates?

Actually for cartesian, we should only take into account +to_meter and apply it to the 3 axis (doesn't make sense to use one unit for XY, and another one for Z)

That would be true, yes.

@rouault

This comment has been minimized.

Member

rouault commented Jun 21, 2018

You could in theory have geocentric CRS with non-meter unit. The following is perfectly valid

                    "GEODCRS[\"WGS 84\",\n"
                    "    DATUM[\"WGS_1984\",\n"
                    "        ELLIPSOID[\"WGS 84\",6378137,298.257223563,\n"
                    "            LENGTHUNIT[\"metre\",1]]],\n"
                    "    PRIMEM[\"Greenwich\",0,\n"
                    "        ANGLEUNIT[\"degree\",0.0174532925199433]],\n"
                    "    CS[Cartesian,3],\n"
                    "        AXIS[\"(X)\",geocentricX],\n"
                    "        AXIS[\"(Y)\",geocentricY],\n"
                    "        AXIS[\"(Z)\",geocentricZ],\n"
                    "        LENGTHUNIT[\"kilometre\",1000]]\n"
@kbevers

This comment has been minimized.

Member

kbevers commented Jun 21, 2018

I was hoping that you had a real world example and not just theory :-) I find it hard to believe that anyone would define a cartesian system that isn't in meters. Oh well, we might as well support that sort of craziness. In that case I think the correct thing to do is to apply +to_meter in the prepare/finalize functions as you suggest.

If people actually do this they should know what they are doing and not pass the cartesian coordinates into helmert uncritically for instance.

@rouault

This comment has been minimized.

Member

rouault commented Jun 21, 2018

a real world example and not just theory :-)

wasn't that a real world example ? All sort of crazy things can come up with WKT, so if we say we support WKT, we must be ready for them (or error out for cases we don't want to support)

and not pass the cartesian coordinates into helmert uncritically for instance.

Not sure to understand what you mean. If people specify a SRS with geocentric with unit = km and pass X,Y,Z values in km, then we can properly convert into metre internally , and helmert will be fine
Not sure to understand

@kbevers

This comment has been minimized.

Member

kbevers commented Jun 21, 2018

wasn't that a real world example ? All sort of crazy things can come up with WKT, so if we say we support WKT, we must be ready for them (or error out for cases we don't want to support)

I was looking for a system defined by a national mapping agency or something to that effect. But sure, if WKT and the standard behind allow it we should support it.

Not sure to understand what you mean. If people specify a SRS with geocentric with unit = km and pass X,Y,Z values in km, then we can properly convert into metre internally , and helmert will be fine
Not sure to understand

I meant to say that you should pay attention to the units of the parameters used with helmert. It would definitely be possible to define helmert parameters for use with a km-units instead of meters for instance. The maths allow that just fine. And if an authority defines a cartesian system using an odd unit it would probably also publish helmert parameters that fit that odd choice of unit. I am not sure what is the best thing to do here.

@rouault

This comment has been minimized.

Member

rouault commented Jun 21, 2018

it would probably also publish helmert parameters that fit that odd choice of unit

For projections parameters, we alredy require meters and degrees everywhere, so for Helmert, we can also require it (if we wanted to accept other units, we could imaginee to accept units to be appended to parameter values, like +x_0=500km). This is a bit different for input / output coordinates that should be expressed in the units mandated by the CRS definition

@kbevers

This comment has been minimized.

Member

kbevers commented Jun 21, 2018

True. The rotation parameters are also required to be a certain type of unit as it is now. Let's go with a requirement of meters as input/output and scale it afterwards in the prepare/finalize functions.

@rouault

This comment has been minimized.

Member

rouault commented Jun 21, 2018

Actually looking at pj_unitconvert.c source code, it does support numeric factors and this is tested by test/gie/unitconvert.gie

$ echo "1 1 0" | src/cct +proj=unitconvert +xy_in=2.0 +xy_out=4.0
       0.5000         0.5000        0.0000           inf

Should that be documented in https://proj4.org/operations/conversions/unitconvert.html ?

@kbevers

This comment has been minimized.

Member

kbevers commented Jun 21, 2018

Oh yeah, forgot that I put that in a while ago. It should definitely be in the docs!

rouault added a commit to rouault/proj.4 that referenced this issue Jun 21, 2018

Add support for deg, rad and grad in unitconvert (fixes OSGeo#1052), …
…and document that it supports numeric factors (refs OSGeo#1053)

rouault added a commit to rouault/proj.4 that referenced this issue Jun 21, 2018

Add support for deg, rad and grad in unitconvert (fixes OSGeo#1052), …
…and document that it supports numeric factors (refs OSGeo#1053)

rouault added a commit to rouault/proj.4 that referenced this issue Jun 21, 2018

Add support for deg, rad and grad in unitconvert (fixes OSGeo#1052), …
…and document that it supports numeric factors (refs OSGeo#1053)

rouault added a commit to rouault/proj.4 that referenced this issue Jun 21, 2018

Add support for deg, rad and grad in unitconvert (fixes OSGeo#1052), …
…and document that it supports numeric factors (refs OSGeo#1053)

rouault added a commit to rouault/proj.4 that referenced this issue Jun 21, 2018

kbevers added a commit that referenced this issue Jun 22, 2018

Merge pull request #1054 from rouault/add_angular_units_to_unitconvert
Add support for deg, rad and grad in unitconvert (fixes #1052), and document that it supports numeric factors (refs #1053)

kbevers added a commit that referenced this issue Jun 22, 2018

Merge pull request #1055 from rouault/geocent_cart_to_meter
Make +proj=geocent and +proj=cart take into account +to_meter (relates to #1053)
@rouault

This comment has been minimized.

Member

rouault commented Jun 22, 2018

All aspects raised by this tickets have now been covered. Closing

@rouault rouault closed this Jun 22, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment