-
Notifications
You must be signed in to change notification settings - Fork 802
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
Add invertible Horner polynomials #3133
Conversation
This enables the user to create horner polynomial transformations in the real number space without the need to supply "inv" parameters: "+inv_origin", "+inv_u" and "+inv_v". We can get the inverse transformation by only using the "fwd" parameters and solving a system of equations iteratively. The system is usually solved in a very small number of iterations. A new optional parameter "+inv_tolerance" is added that can make the iterative process stop earlier if a certain coordinate accuracy is met. For example "+inv_tolerance=0.1" will make the iterative process stop when the calculated coordinates differ less than 0.1 meters, if the units of the transformation destination is meters.
I will also try to create docs for this. |
This is due to a false positive warning. Also add const to some variables that do not change.
In the case of inversible horner, return error values if the iterations required for convergence are too high.
I also believe there are errors in the horner documentation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't really comment on the maths but here's a few suggestions for improving the docs. I hope @busstoptaktik can find the time to give this a proper review at some point. He has the expertise in this part of the code base.
Move +inv_u, +inv_v to optional parameters in horner docs Make some consistency changes for matrices and default values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doc changes looks good to me!
This enables the user to create horner polynomial transformations in the complex number space without the need to supply "inv" parameters: "+inv_origin" and "+inv_c". Also added a unit test and respective docs
Just added complex inversion too :) |
Use a similar algorithm as the normal path. This enables us to not use std::pow at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Certainly a useful addition to the Horner code. See, however, my comments regarding the probable un-soundness of evaluating the polynomium by brute force, rather than Horner's scheme.
Also, IOGP Publication 373-7-2 – Geomatics Guidance Note number 7, part 2 – September 2019 has a very clear section on the criteria for polynomium reversibility. It would probably be a good thing to quote and refer extensively from/to that in the documentation updates. For example, the Geomatics Guidance Note says:
Polynomial reversibility
Approximation polynomials are in a strict mathematical sense not reversible, i.e. the same polynomial coefficients cannot be used to execute the reverse transformation
However, under certain conditions, described below, a satisfactory solution for the reverse transformation may be obtained using the forward coefficient values in a single step, rather than multiple step iteration. If such a solution is possible, in the EPSG Dataset the polynomial coordinate transformation method is classified as a reversible polynomial of degree n.
A (general) polynomial transformation is reversible only when the following conditions are met.
- The coordinates of source and target evaluation point are (numerically) the same.
- The unit of measure of the coordinate differences in source and target coordinate reference system are the same.
- The scaling factors applied to source and target coordinate differences are the same.
- The spatial variation of the differences between the coordinate reference systems around any given location is sufficiently small.
Your code solves a more general problem, and hence extends on the EPSG sense of a reversible polynomium - it probably would be a good idea to explicitly state this in the documentation.
- repurpose has_only_fwd variable to has_inv - optimize polynomial inversion - refactor horner evaluation code to reusable functions
@busstoptaktik i will make a separate commit for the docs.
So, i will update the docs, so as to make it clear this is an iterative inverse transformation and not a "reversible polynomial" as presented in the publication. Another thing i realized when i read the section of the polynomial transformations in the publication, is that:
|
Cite IOGP Publication on reversible polynomials and the difference with our general solution
@busstoptaktik i added the doc reference to the publication. Also i refactored the code as requested in the review. |
As discussed in OSGeo#3133 the existing functions are now a bit complicated and can be refactor to simpler ones. Checks for invalid direction are now unnecessary, as the forward and reverse paths are only accessible through fwd4d and inv4d function callbacks in the projection object. Checks for a nullptr HORNER transformation object are too unnecessary as enough checks are done in the projection object construction.
As discussed in #3133 the existing functions are now a bit complicated and can be refactor to simpler ones. Checks for invalid direction are now unnecessary, as the forward and reverse paths are only accessible through fwd4d and inv4d function callbacks in the projection object. Checks for a nullptr HORNER transformation object are too unnecessary as enough checks are done in the projection object construction.
This enables the user to create horner polynomial transformations
in the real number space without the need to supply "inv" parameters:
"+inv_origin", "+inv_u" and "+inv_v". We can get the inverse
transformation by only using the "fwd" parameters and solving
a system of equations iteratively. The system is usually solved in a
very small number of iterations.
A new optional parameter "+inv_tolerance" is added that can make the
iterative process stop earlier if a certain coordinate accuracy is
met. For example "+inv_tolerance=0.1" will make the iterative
process stop when the calculated coordinates differ less than
0.1 meters, if the units of the transformation destination is meters.
docs/source/*.rst
for new API