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

Jigsaw - Residual Percentiles have changed #4655

Closed
lwellerastro opened this issue Oct 14, 2021 · 1 comment · Fixed by #4676
Closed

Jigsaw - Residual Percentiles have changed #4655

lwellerastro opened this issue Oct 14, 2021 · 1 comment · Fixed by #4676
Assignees
Labels
bug Something isn't working Products Issues which are impacting the products group
Milestone

Comments

@lwellerastro
Copy link
Contributor

lwellerastro commented Oct 14, 2021

ISIS version(s) affected: 6.0.0

Description
The Residual Percentiles and Residual Box Plot in the bundleout.txt file has dramatically changed from the last version of isis.

How to reproduce
Compare all output from isis5.0.2 jigsaw to all output from isis6.0.0. I don't think the type of solution matters as I saw this while solving for camera accelerations via Kaguya TC, but tested on a smaller Themis IR data set solving for camera and spacecraft with errorprop turned on (re-using old test data). The only difference in the isis5.0.2 and isis6.0.0 output (including output network files) was the Residual Percentiles/Box Plot reported under isis6.0.0 are very much larger than what is reported in isis5.0.2 and don't correlate with what is reported in the residuals.csv file.

There is a small test network under /work/users/lweller/Isis3Tests/Jigsaw/Percentiles/. See proc.scr to see command lines for both versions of isis as well as the diff tests run. Note that the images in the input jigsaw fromlist reside in another directory.

Since it's small enough, here are the Residual Box Plot differences in the two runs (I've omitted the lengthier Percentiles output here):

diff isis5.0.2_bundleout.txt isis6.0.0_bundleout.txt
<                         minimum:   -1.204
<                      Quartile 1:   -0.110
<                          Median:   -0.001
<                      Quartile 3:   +0.103
<                         maximum:   +1.490
---
>                         minimum:  -17.194
>                      Quartile 1:   -1.576
>                          Median:   -0.010
>                      Quartile 3:   +1.469
>                         maximum:  +21.281

The maximum residual in residuals.csv is 1.8 so the isis5.0.2 output makes better sense.

Note that this output is not useful or even relevant to me - I'm simply reporting a bug. I just happened to notice how large the values were when I was scrolling through a bundleout file having just seen my maximum residual was less than 2 (the min/max box plot values where >900 for my much larger Kaugya TC data set then the test data I provided and reported on above).

Although the test case above touches a few different parameters, I don't know if there are other differences in the isis versions other what's reported. It's possible something else differs for say multi- sensor solutions. I'm just suggesting I haven't exhausted the possibilities.

@lwellerastro lwellerastro added the Products Issues which are impacting the products group label Oct 14, 2021
@jessemapel jessemapel added the bug Something isn't working label Oct 15, 2021
@jessemapel
Copy link
Contributor

jessemapel commented Oct 15, 2021

This appears to be coming from how we had to abstract out some of the CSM vs ISIS image stuff. For ISIS images we weight everything based on the pixel size but we can't do that for CSM so we just weight them all 1.

For whomever works this, it looks like IsisBundleObservation::computeObservationValue is not properly backing out the division by 1.4. Here's the old logic for this part. We should be adding deltaX / pixelPitch and deltaY / pixelPitch to the residual PDF. During the CSM integration we split this into two steps so that we could abstract it for CSM. The first step computes deltaX / (1.4 * pixelPitch) and deltaY / (1.4 * pixelPitch). The second step divides again by pixelPitch. This results in ( deltaX / (1.4 * pixelPitch) ) / pixelPitch = deltaX / (1.4 * pixelPitch**2) which is incorrect. Instead the second step should be multiplying 1.4 so it does ( deltaX / (1.4 * pixelPitch) ) * 1.4 = (1.4 * deltaX) / (1.4 * pixelPitch) = deltaX / pixelPitch which matches before.

This is only used for the box plot reporting and nothing else, so this isn't causing bad solutions, just bad reporting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Products Issues which are impacting the products group
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants