Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Adding jigsaw examples #4734
Adding jigsaw examples #4734
Changes from 3 commits
04f873b
0b1b387
f776bca
1e5a826
28190b2
3b8a2cd
98dfb5e
9029647
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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'm not sure if this paragraph is entirely correct. You can certainly still solve for all of these things, as long as you have enough observations for the number of unknowns, but they will all be determined relative to the spacecraft. So, the ground solution relative to the spacecraft will be correct, but the correctness of the ground solution relative to the rest of the body is unknown. You can come back and use tools like PC align to correct for this type of error.
There is likely a more technical set of terms for this, I'll reach out to Brent.
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 guess I am under the impression that within ISIS, if you do not have ground in a network then solving for spacecraft position is not really worth the effort. But I would really appreciate more certain terminology, let me know if you hear from Brent
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.
and does not require parameterization
what does this mean? Is this saying that solving for twist is optional?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 mean twist is a flag and does not have a corresponding uncertainty estimation requirement. I will update to be more clear.
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.
Change
higher derivative parameters
tohigher order parameters
. We're still just computing first partials with respect to all of these parameters, the acceleration is a higher order term in the pointing polynomials.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.
This paragraph is incorrect. The bundle solves for a polynomial regardless of
overexisting
or not. When you do not haveoverexisting
on, the bundle starts by fitting a polynomial to the a-priori ephemerides and then just uses that polynomial to compute all ephemerides. If you enableoverexisting
, it instead saves the a-priori ephemerides and begins with a zero polynomial. The a-priori ephemerides are then added to ephemerides computed from the polynomial that the bundle solves for. Here's a simple example:overexisting off
Initial data : {(1,1), (2, 4), (3, 9), (4, 16)}
fitting a linear polynomial we get f(x) = 5x - 5
then, we throw the initial data away and we now compute all ephemerides using our function, f. So, {(1, f(1)), (2, f(2), ...}
The bundle will compute corrections to the coefficient of f.
overexisting on
Initial data : {(1,1), (2, 4), (3, 9), (4, 16)}
We start with a zero polynomial, f(x) = 0x + 0
Now, we keep the original data and add the result of f to it. So, {(1, 1+f(1)), (2, 4+f(2)), (3, 9+f(3)), ...}.
The bundle still computes corrections to our function f and because the initial data is just a constant offset, it doesn't impact our partial computations in the bundle.
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.
Add a not above that you have
update=no
because you are still evaluating the solution and do not want to apply it to your images just yet.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 would avoid the word
smith
as it could be confused withsmithed
kernels that are produced from bundle adjusted images. I would instead use something likerefined
orimprove
.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 would bold
after
instead of using all caps. In the brief description bold is used for emphasis but here all caps are.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 switched because I was having trouble with bold not showing up, I will switch it to be consistent (which is a good suggestion) and hopefully I can figure out why the bold is not showing when I build the docs locally.
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.
See my comment on the previous example about relative solutions vs grounded solutions. You can 100% still solve for these things and they will be relatively correct without ground.
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 will just delete the sentence "Radius and spacecraft position are two parameters which can only be solved for when ground points are present in the bundle solution since they are concerned with absolute positioning"
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.
Be careful correlating these too closely. We've had users confused about the sigmas not being hard constraints. They are a-priori uncertainties, or standard deviations, not hard constraints. So, it's expected that some amount of points have corrections higher than their sigmas, in fact in a perfect world, we'd expect about a third of the points to have corrections higher than 1 sigmas.
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.
See my comments in the previous example. using
overexisting
oroverhermite
actually takes more memory than just solving for a polynomial as we have to store the a-priori ephemerides and the polynomial.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 would recommend not changing any parameters besides
update
in the final solution. You should check that you like the whole solution before you save it. It's kind of a hassle, but there's an issue open already to allow us to apply a previous solution independently. #4474There 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.
It would be great to provide an example of a table that has been updated and one that hasn't. Like a before and after.