Calculate maneuver times and angle from cmds, replace maneuver summary file#408
Calculate maneuver times and angle from cmds, replace maneuver summary file#408
Conversation
|
This strategy is just to reuse the maneuver calculation code that was being called in hopper and fiddle with that data structure. An alternative path would be to just make a similar structure with kadi.commands, or reorganize more significantly. |
|
@taldcroft Do you think it makes sense to use the hopper maneuver code for this? Or would it make more sense to retire hopper in general as a nice idea but mostly more than we needed and use some new maneuver code built with kadi.commands and replace the hopper attitude checking code with something simpler? |
|
Using hopper is fine for now. |
|
Sorry for going back and forth, but I looked at the code and again thought there must be a better way. I managed to find the old thread "Issue with MCC loading maneuver summary for AUG1312A" with Eric's summary of the flight calculation: So why not have starcheck use exactly the OBC-normalized quaternion from above. Then there is no need to check for consistency because we are using the right answer. We could leave in the check about normalization, but I ran that check on the entire mission with kadi commands and it has never failed. Backstop just isn't going to deliver badly-normalized quaternions and we rely on those all over the place without sanity checks. |
|
In theory we could add a kwarg to the |
|
"So why not have starcheck use exactly the OBC-normalized quaternion from above." That's basically what I was thinking but I was leaning toward doing it in a future release. The issue is that, as you noted in your on-the-side review, what starcheck is using seems to not really be nailed down... it seems to use something different for reference attitude in each method, so I thought it would be more of a little project to review and make uniform. But we could do this now. Though with regard to your kadi cmds review --- that was of approved products and not including early mission. |
|
I also did all mission commands in the archive and the max norm error was around 3e-7. But interestingly at this level the RA/dec/roll offsets can be substantial, 10's of arcsec. For more recent times the offsets were sub-arcsec. |
|
OK, we can defer making improvements in the way starcheck works to later. I'll approve this once you summarize the outcome of the functional testing in the description. |
| [m['final']['q1'], m['final']['q2'], | ||
| m['final']['q3'], m['final']['q4']])) | ||
|
|
||
| angle = np.degrees(2 * np.arccos(q1.q.dot(q2.q))) |
There was a problem hiding this comment.
One thing I learned in quick functional test output review is that the maneuver angle we have been using from the matlab tools is not the great circle angle (sphere_dist) and is instead more accurately reflected as this (not sure what you call this angle - rotation matrix angle?). I don't know if that is actually interesting at all for acquisition probability.
There was a problem hiding this comment.
For this maneuver from DEC2622A, the maneuver from 44926 to 44925 has a maneuver 'angle' of 165.05 degrees, but a sphere_dist of 14.6 deg.
OBSID: 44926
RA, Dec, Roll (deg): 93.000000 23.000000 271.874615
Dither: ON Y_amp= 8.0 Z_amp= 8.0 Y_period=1000.0 Z_period= 707.1
BACKSTOP GUIDE_SUMM OR DOT TLR
MP_TARGQUAT at 2023:001:10:27:59.567 (VCDU count = 7072498)
Q1,Q2,Q3,Q4: -0.36518414 -0.59294570 0.41533499 0.58528010
MANVR: Angle= 74.61 deg Duration= 1655 sec Slew err= 51.3 arcsec End= 2023:001:10:55:40
====================================================================================
OBSID: 44925
RA, Dec, Roll (deg): 108.597458 20.909120 70.938652
Dither: ON Y_amp= 8.0 Z_amp= 8.0 Y_period=1000.0 Z_period= 707.1
BACKSTOP GUIDE_SUMM OR DOT TLR
MP_TARGQUAT at 2023:001:11:50:04.357 (VCDU count = 7091716)
Q1,Q2,Q3,Q4: 0.45300842 0.37715544 0.71183683 0.38187101
MANVR: Angle= 165.05 deg Duration= 2861 sec Slew err= 27.6 arcsec End= 2023:001:12:37:50
There was a problem hiding this comment.
This is a great point and one I hope we'll remember. Fundamentally the way I would think about this is:
dq = q1.dq(q2) # quat getting from q1 to q2 (or vice versa, I never remember)
Then dq.q[3] = cos(alpha / 2) where alpha is the rotation angle about q[0:2].
I'm assuming that q1.q .dot (q2.q) is the same as dq.q[3]. (And I empirically confirmed this for one random case).
Q56. How do I convert a rotation axis and angle to a quaternion?
----------------------------------------------------------------
Given the rotation axis and angle, the following algorithm may be
used to generate a quaternion:
------------------------------------------------
sin_a = sin( angle / 2 )
cos_a = cos( angle / 2 )
q -> x = axis -> x * sin_a
q -> y = axis -> y * sin_a
q -> z = axis -> z * sin_a
q -> w = cos_a
quaternion_normalise( q );
------------------------------------------------
I almost wonder if we should add a method to Quat like Quat.rotation_angle(self, q2).
|
Another side effect of this process is that all maneuvers appear to end 10 seconds later than the maneuver summary had previously reported. I don't actually know which one is a closer estimate. With the new estimate, the monitor window checks in the nov2822 test products didn't pass. So need to figure out if we update the monitor window timing checks or subtract 10 seconds from all of the maneuver end times in this PR. |
|
That's the standard 10 sec delta in the man summary right? The backstop timing is the truth which we should use, so updating the mon window timing would seem like the way to go. |
|
I just wasn't sure which was actually right as there is no maneuver end time in backstop and we've been using the maneuver summary end time for quite a while. It looks like the new derived times more closely match kadi manvr events, so that seems fine and agrees with your strategy. |
jeanconn
changed the title
|
I reran the tests and added one with big dither to go through those timing checks too (which seemed OK). |
| [m['final']['q1'], m['final']['q2'], | ||
| m['final']['q3'], m['final']['q4']])) | ||
|
|
||
| # Calculate maneuver angle using code borrowed from kadi |
There was a problem hiding this comment.
I updated the angle calculation to just use that 4th component -- which seems to work and was already numerically stable here with the logic from kadi commands even if q2 == q1?
|
@jeanconn - I was filling in the interface impact and realized that we (and especially Eric) find some value in clicking through to the maneuver summary file. Even if it is no longer used for starcheck review maybe we should keep that link there? |
|
This is great work with a very thorough job of testing! Ready for approval and merge once we decide on the maneuver summary file. |
|
Sure, no problem to put the maneuver summary back as the annotated HTML. There is some value in we know this PR was tested (at this point) without even reading the file, and adding it back just as the HTML is benign (though I'll rerun testing and review after those changes). |
|
Instead of re-running testing, since @taldcroft had already reviewed those diffs in detail, I did incremental output diffs So the new *diff2.txt files show the expected output that MANVR is back, relative to the last round of test outputs. And inspection of a *test2.html file shows the MANVR link works. |
2 participants
Description
Calculate maneuvers from backstop cmds and replace maneuver summary as source of maneuver times and angle.
This fixes the known issue that the maneuver summary file is incorrect for observations with nonzero MANSTART.
Fixes #409
Interface impacts
Starcheck text output changed slightly, no impact to parsing tools:
Testing
Tested with ska3-matlab 2023.4rc6 on fido, though for these changes no special test setup should be required.
Unit tests
Functional tests
Used the "new" regression test set we used to review #402 plus two weeks with MANSTART values that caused issues with maneuver end times in flight starcheck
Output in https://icxc.cfa.harvard.edu/aspect/test_review_outputs/starcheck-pr408/
Test Review
@taldcroft looked through all the diffs and found only expected changes.