-
Notifications
You must be signed in to change notification settings - Fork 32
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
Quaternion sign flip for LROC NAC #603
Comments
I can share the cube also (just let me know). Too big to post. More discussion with Oleg: Also, these are close to the lunar north pole, so it could be a pole issue. The conversion from rotation to quaternion has to make a choice about sign, and not sure what makes it pick one sign vs another, when close to some extreme like the pole. |
@thareUSGS If you can post the cube label, that should be enough to get an ISD out of ALE. Oleg mentioning this is at the polls makes more sense |
There is no label, this has to come directly from the .img. That one is here: https://wms.lroc.asu.edu/lroc/view_lroc/LRO-L-LROC-2-EDR-V1.0/M124343339LE If a sanity check with mapprojection is needed with the logic that USGSCSM now provides, the problematic area in the clip above is in the middle of the image (where it is brightest) and the artifact shows up along the entire image width. |
from naif docs
I'm pretty confident that this is the issue. The sign of the scalar portion "should" flip at some point in the ISD, but instead it hits 0 and flips the rest of the signs in order to stay positive, e.g.:
It seems like we could either 1. do post-processing to flip the signs in the ISD, 2. modify the interpolation code to choose between the quaternion (q) or its inverse (q*) in a way that minimizes the distance between the two quaternions we're interpolating between, or 3. something else? Any thoughts? |
This should be fixed at the source, I believe, not later, during interpolation. The produced ISD should be correct, with consistent sign. The logic I used in ASP to fix this is to first identify which of the four coordinates of a quaternion is largest in magnitude, as that one is the most likely to be preserved. Then it is ensured that the sign of that coordinate is consistent throughout the sequence, by flipping or not each quaternion. |
I'm going to be a pain in the butt here and push for more discussion: Correct and consistent with respect to which standards? It would seem that the ISDs are correct and consistent with the quaternion style used in the NAIF utilities, and it seems to me that flipping the signs would make the ISDs less consistent with existing standards*. It seems to me that these are perfectly valid quaternions, and that the issue is with the interpolation process, not with the way that quaternions are specified. I would say that the interpolation logic contains a bug in which it cannot handle a certain form of valid quaternions, and that we should address that bug rather than forcing quaternions to be specified in one valid form rather than another valid form. * I am definitely conflating "single design decision of m2q" with "standard for quaternions." |
The interpolation logic in the usgscsm library does not know about quaternions. It interpolates into a matrix of size M x N, where M is the number of values, and N = 3 for positions and N = 4 for quaterions. Fixing the interpolation logic to handle discontinuous arrays and do flips on demand is likely not a good idea. Not to mention that ASP's jitter solver has its own interpolation and several optimization algorithms that dearly would like to assume nice and smooth input data. |
Based on more comments from this discussion within the pull-request #612, it looks like we should proceed to fix any flip within ALE (and not isd_generate.py, which was really a work-around). Jay also pointed to this stackoverflow (not a great resource), but that discussion provides the same recommendation (fix the flip). Summary: To move this forward, I do think we need to implement this check and update in ALE (and reject the #612 PR for isd_generate.py which wouldn't be needed). |
I believe naif chioses one convention in order to ensure some
predictability, not because one sign is better than another.
For linescan cameras, it makes sense to supercede thet convention with
another one which says there must be continuity along the pose sequence.
…On Sat, Aug 3, 2024, 12:01 AM Austin Sanders ***@***.***> wrote:
from naif docs
<https://naif.jpl.nasa.gov/pub/naif/toolkit_docs/C/cspice/qxq_c.html>
m2q_c always returns a quaternion with scalar part greater than or equal to zero.
I'm pretty confident that this is the issue. The sign of the scalar
portion "should" flip at some point in the ISD, but instead it hits 0 and
flips the rest of the signs in order to stay positive, e.g.:
[
0.00014084959512966424, <<< positive, decreasing
-0.06988009226932775,
0.9799806108855965,
0.18642627270348733
],
[
5.113202307559794e-05, <<< positive, decreasing, about to flip sign
-0.0698602605370718,
0.9799805775003425,
0.18643392691595587
],
[
3.8327611438555095e-05, <<< Should be negative, but flips all signs to keep scalar positive
0.06983938596985084,
-0.9799806386052183,
-0.1864414295623614
],
[
0.0001291419108749148,
0.06981977990312137,
-0.979980865507725,
-0.1864475392580358
],
It seems like we could either 1. do post-processing to flip the signs in
the ISD, 2. modify the interpolation code to choose between the quaternion
(q) or its inverse (q*) in a way that minimizes the distance between the
two quaternions we're interpolating between, or 3. something else?
Any thoughts?
—
Reply to this email directly, view it on GitHub
<#603 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKDU3HDUH5BFHPMZ34KFODZPPXRTAVCNFSM6AAAAABIAAXRRWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRWGEZTSOJRGE>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Oleg noticed an issue with some recent LROC NAC testing at the pole. The file M124343339LE.lev1.8bit.json seems to have a problem where the quaternions (used for orientation) have inconsistent signs. Example (line ~2193):
Oleg states, each quaternion alone is fine, as it defines the same rotation matrix with any sign. The problem is that the CSM linescan model interpolates between them, and that results in orientations being junk when the quaternions change sign.
He also prepared an image example which is the result after a temporary fix in ASP. Both of these are after mapprojection with ASP. The raw image does not have this artifact, it comes from the camera.
M124343339LE.lev1.8bit.json
The text was updated successfully, but these errors were encountered: