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

Enhancing unitconvert to support arbitrary conversion factor #1053

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

Enhancing unitconvert to support arbitrary conversion factor #1053

rouault opened this issue Jun 21, 2018 · 15 comments

Comments

@rouault
Copy link
Member

@rouault 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
Copy link
Member Author

@rouault rouault commented Jun 21, 2018

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

@kbevers
Copy link
Member

@kbevers 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
Copy link
Member Author

@rouault 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
Copy link
Member Author

@rouault 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
Copy link
Member Author

@rouault 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
Copy link
Member

@kbevers 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
Copy link
Member Author

@rouault 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
Copy link
Member

@kbevers 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
Copy link
Member Author

@rouault 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
Copy link
Member

@kbevers 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
Copy link
Member Author

@rouault 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
Copy link
Member

@kbevers 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
Copy link
Member Author

@rouault 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
Copy link
Member

@kbevers 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 that referenced this issue Jun 21, 2018
…and document that it supports numeric factors (refs OSGeo#1053)
rouault added a commit to rouault/PROJ that referenced this issue Jun 21, 2018
…and document that it supports numeric factors (refs OSGeo#1053)
rouault added a commit to rouault/PROJ that referenced this issue Jun 21, 2018
…and document that it supports numeric factors (refs OSGeo#1053)
rouault added a commit to rouault/PROJ that referenced this issue Jun 21, 2018
…and document that it supports numeric factors (refs OSGeo#1053)
rouault added a commit to rouault/PROJ that referenced this issue Jun 21, 2018
…s to OSGeo#1053)
kbevers added a commit that referenced this issue Jun 22, 2018
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
Make +proj=geocent and +proj=cart take into account +to_meter (relates to #1053)
@rouault
Copy link
Member Author

@rouault 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
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants