WIP: Use Horner's method in `Horner` function 1/2
Summary
First MR of a series about simplifying and improving SARCalibration
design and performances.
This MR is about using Horner's method on Y axis in SarParametricMapFunction::Horner()
function.
Rationale
Horner's method is much faster than using std::pow()
multiple times, and it offers a better numerical precision.
Implementation Details
The MR will be done through 2/3 commits.
First commit: the baseline enforces numerical errors
Before fully using Horner's method, let improve the precision.
Here, point
can be an array of float
s, we divide its components with 2 double
s.
And we do computations with other double
s. Mixing float
s and double
s in
computations will trigger conversions and prevent the compiler from correctly
vectorizing the operations.
Hence, it's better to cache point[0|1] / W|H
in two double
s but now
raTvSarParametricMapFunctionToImageFilter
fails...
My understanding is that the new computation should be more precise than the initial one, and I strongly suspect that the baseline enforces a small numerical error.
Can anybody else confirm my analysis? Also, can some one check whether some SARcalibration cases are improved?
Final commit: Implement Horner's method on Y's as well
Once we agree about the first part regarding the precision improvement, I'll fully implement Horner's method (commented as of now)
Copyright
The copyright owner is CS Group FRANCE and has signed the ORFEO ToolBox Contributor License Agreement.
Check before merging:
- All discussions are resolved
- At least 2
votes from core developers, no vote. - The feature branch is (reasonably) up-to-date with the base branch
- Dashboard is green
- Copyright owner has signed the ORFEO ToolBox Contributor License Agreement
- Optionally, run
git diff develop... -U0 --no-color | clang-format-diff.py -p1 -i
on latest changes and commit
Merge request reports
Activity
Note, regarding the precision issue, here is a little excerpt that shows computation before (float mixed with doubles), and after (only doubles). The term that is used either as float or double is the one after the multiplication: 0.49
- We can observe a better approximation of 0.49 on the "double" line
- As a consequence, the accumulated initial value has less noise.
- We can expect the final and expected value to be: (320 * 0.49 + 280) * 0.49 +10 = 224.032
- We see that the "double" path clearly ends up with a much better approximation
- The "float" path is wrong at the 9th digit (the 6th after the comma) while the test requires a conformance to 1e-12 (I haven't checked if this is an absolute or a relative precision)
This is what makes me think the baseline shall be fixed.
float: x=3 : 320.00000000000056843418860808 <- 0 * 0.4900000095367431640625 + 320.00000000000056843418860808 double: x=3 : 320.00000000000056843418860808 <- 0 * 0.489999999999999991118215802999 + 320.00000000000056843418860808 float: x=2 : 436.800003051758153560513164848 <- 320.00000000000056843418860808 * 0.4900000095367431640625 + 280.000000000000056843418860808 double: x=2 : 436.800000000000295585778076202 <- 320.00000000000056843418860808 * 0.489999999999999991118215802999 + 280.000000000000056843418860808 float: x=1 : 224.032005661010913399877608754 <- 436.800003051758153560513164848 * 0.4900000095367431640625 + 9.99999999999997690736108779674 double: x=1 : 224.032000000000124373400467448 <- 436.800000000000295585778076202 * 0.489999999999999991118215802999 + 9.99999999999997690736108779674
Edited by Luc Hermitteadded 576 commits
-
a9b556b2...bc0cb8dd - 575 commits from branch
orfeotoolbox:develop
- 1661bffd - Merge branch 'develop' of https://gitlab.orfeo-toolbox.org/orfeotoolbox/otb...
-
a9b556b2...bc0cb8dd - 575 commits from branch
It seems a few other tests needs to be updated. Strangely enough, the only difference is on mac...
https://cdash.orfeo-toolbox.org/viewTest.php?onlyfailed&buildid=65297
This is because the Mac build is currently the only job that runs the "LargeInput" tests, including the tests including TerraSarX product data (e.g.
TERRASARX/PANGKALANBUUN/IMAGEDATA/IMAGE_HH_SRA_stripFar_008.cos
)Edited by Cédric Traizet
changed milestone to %7.4.0
mentioned in merge request !836 (merged)
Closing, this has been merged in the release-7.4 branch in !836 (merged)