-
Notifications
You must be signed in to change notification settings - Fork 33
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
Update isd_generate.py to support optional function to ensure that instrument pointing quaternions do not change sign #612
base: main
Are you sure you want to change the base?
Conversation
implement an optional function to ensure that instrument pointing quaternions do not change sign.
parser.add_argument( | ||
"-f", "--fix_quaternion", | ||
action="store_true", | ||
help="There are rare cases when a stream of quaternion flip their signs. This option will " | ||
"fix those to be consistent. This takes after a NASA ASP function to do the same. " | ||
) |
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.
Are there cases where you wouldn't want this on?
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.
Good question. I think @oleg-alexandrov would state this should always be run to fix for all images (that assumes I am doing this correctly). But perhaps we have a flag that turns this function off instead of on (implying we would always run it unless the flag stops it).
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.
Weighing in here with a +1 to @oleg-alexandrov's comment below. Always on.
This must run always. This is fix for quartertions having two solutions for
any one rotation. In rare cases one may want the option of not doing this.
…On Wed, Sep 4, 2024, 11:56 PM Trent Hare ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In ale/isd_generate.py
<#612 (comment)>:
> + parser.add_argument(
+ "-f", "--fix_quaternion",
+ action="store_true",
+ help="There are rare cases when a stream of quaternion flip their signs. This option will "
+ "fix those to be consistent. This takes after a NASA ASP function to do the same. "
+ )
Good question. I think @oleg-alexandrov
<https://github.com/oleg-alexandrov> would state this should always be
run to fix for all images (that assumes I am doing this correctly). But
perhaps. we have a flag that turns this function off instead of on.
—
Reply to this email directly, view it on GitHub
<#612 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKDU3GH274J7UL3VRIR2MDZU5XYLAVCNFSM6AAAAABM4WMXRSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEOBRGMYTMNJQGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
To add, this fix must be deep in ALE, in the linescan driver. That some
quarternions flip sign is a limitation of the math. No tools will break if
the sign is regularized.
Nothing is gained by allowing a flip, as any one quarterion is equally good
with any sign.
On Thu, Sep 5, 2024, 3:04 AM Oleg Alexandrov ***@***.***>
wrote:
… This must run always. This is fix for quartertions having two solutions
for any one rotation. In rare cases one may want the option of not doing
this.
On Wed, Sep 4, 2024, 11:56 PM Trent Hare ***@***.***> wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In ale/isd_generate.py
> <#612 (comment)>:
>
> > + parser.add_argument(
> + "-f", "--fix_quaternion",
> + action="store_true",
> + help="There are rare cases when a stream of quaternion flip their signs. This option will "
> + "fix those to be consistent. This takes after a NASA ASP function to do the same. "
> + )
>
> Good question. I think @oleg-alexandrov
> <https://github.com/oleg-alexandrov> would state this should always be
> run to fix for all images (that assumes I am doing this correctly). But
> perhaps. we have a flag that turns this function off instead of on.
>
> —
> Reply to this email directly, view it on GitHub
> <#612 (comment)>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAKDU3GH274J7UL3VRIR2MDZU5XYLAVCNFSM6AAAAABM4WMXRSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEOBRGMYTMNJQGA>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
|
We need to create a new PR that moves this code into the ALE mixing |
Implement an optional function to ensure that instrument pointing quaternions do not change sign.
This is really more to continue the discussion under issue #603 . I understand that NAIF is supposed to return the quaternions with flipped signs, but this seems to be breaking our applications. ASP has been updated to account for this issue. This example function for isd_generate.py, following ASP's logic, could also be used to "clean" the ISD for other applications like SOCET GXP.
Licensing
This project is mostly composed of free and unencumbered software released into the public domain, and we are unlikely to accept contributions that are not also released into the public domain. Somewhere near the top of each file should have these words: