Calculate maneuver times and angle from cmds, replace maneuver summary file

[jeanconn]

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:

• Maneuver end time changed by 10 seconds (new value is correct)

Testing

Tested with ska3-matlab 2023.4rc6 on fido, though for these changes no special test setup should be required.

Unit tests

• [x] No unit tests

Functional tests

Used the "new" regression test set we used to review https://github.com/sot/starcheck/pull/402 plus two weeks with MANSTART values that caused issues with maneuver end times in flight starcheck

jeanconn-fido> more run.sh
jeanconn-fido> more run.sh
# Long week and IR Zone holds
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2022/DEC2622/oflsa -out dec2622a_test
/proj/sot/ska3/flight/bin/starcheck -dir /data/mpcrit1/mplogs/2022/DEC2622/oflsa -out dec2622a_flight
/proj/sot/ska/bin/diff2html dec2622a_flight.txt dec2622a_test.txt > dec2622a_diff.html

# Monitor star
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2022/NOV2822/oflsa -out nov2822a_test
/proj/sot/ska3/flight/bin/starcheck -dir /data/mpcrit1/mplogs/2022/NOV2822/oflsa -out nov2822a_flight
/proj/sot/ska/bin/diff2html nov2822a_flight.txt nov2822a_test.txt > nov2822a_diff.html

# Maneuver only loads
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2022/OCT2422/oflsb -out oct2422b_test
/proj/sot/ska3/flight/bin/starcheck -dir /data/mpcrit1/mplogs/2022/OCT2422/oflsb -out oct2422b_flight
/proj/sot/ska/bin/diff2html oct2422b_flight.txt oct2422b_test.txt > oct2422b_diff.html

# Replan
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2021/MAY0521/oflsa -out may0521a_test
/proj/sot/ska3/flight/bin/starcheck -dir /data/mpcrit1/mplogs/2021/MAY0521/oflsa -out may0521a_flight
/proj/sot/ska/bin/diff2html may0521a_flight.txt may0521a_test.txt > may0521a_diff.html

# MANSTART in APR0323A
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2023/APR0323/oflsa -out apr0323a_test
/proj/sot/ska3/flight/bin/starcheck -dir /data/mpcrit1/mplogs/2023/APR0323/oflsa -out apr0323a_flight
/proj/sot/ska/bin/diff2html apr0323a_flight.txt apr0323a_test.txt > apr0323a_diff.html

# MANSTART in APR1723A
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2023/APR1723/oflsa -out apr1723a_test
/proj/sot/ska3/flight/bin/starcheck -dir /data/mpcrit1/mplogs/2023/APR1723/oflsa -out apr1723a_flight
/proj/sot/ska/bin/diff2html apr1723a_flight.txt apr1723a_test.txt > apr1723a_diff.html

# JUN1923 week big dither
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2023/JUN1923/oflsa -out jun1923a_test
/proj/sot/ska3/flight/bin/starcheck -dir /data/mpcrit1/mplogs/2023/JUN1923/oflsa -out jun1923a_flight
/proj/sot/ska/bin/diff2html jun1923a_flight.txt jun1923a_test.txt > jun1923a_diff.html

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.

[jeanconn]

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.

[jeanconn]

@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?

[taldcroft]

Using hopper is fine for now.

[taldcroft]

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:

     Q_Mag := ( Q_Targ_Cmd(1)*Q_Targ_Cmd(1) ) +
             ( Q_Targ_Cmd(2)*Q_Targ_Cmd(2) ) +
             ( Q_Targ_Cmd(3)*Q_Targ_Cmd(3) );

     Q_Targ_Cmd(4) := SQRT( ABS(1.0 - Q_Mag ));  # Edit to include the ABS() there
     QUATERNION_NORMALIZE( Q_Targ_Cmd, Q_Targ_Cmd, Q_Norm_OK );

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.

[taldcroft]

In theory we could add a kwarg to the Quat class and Quaternion.normalize to tell it to normalize the input like the OBC does.

[jeanconn]

"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.

[taldcroft]

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.

[taldcroft]

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.

[jeanconn]

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.

[taldcroft]

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.

[jeanconn]

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]

I reran the tests and added one with big dither to go through those timing checks too (which seemed OK).

[taldcroft]

@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?

[taldcroft]

This is great work with a very thorough job of testing! Ready for approval and merge once we decide on the maneuver summary file.

[jeanconn]

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).

[jeanconn]

Instead of re-running testing, since @taldcroft had already reviewed those diffs in detail, I did incremental output diffs

jeanconn-fido> more incremental.sh
# Long week and IR Zone holds
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2022/DEC2622/oflsa -out dec2622a_test2
diff dec2622a_test.txt dec2622a_test2.txt > dec2622a_diff2.txt

# Monitor star
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2022/NOV2822/oflsa -out nov2822a_test2
diff nov2822a_test.txt nov2822a_test2.txt > nov2822a_diff2.txt

# Maneuver only loads
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2022/OCT2422/oflsb -out oct2422b_test2
diff oct2422b_test.txt oct2422b_test2.txt > oct2422b_diff2.txt

# Replan
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2021/MAY0521/oflsa -out may0521a_test2
diff may0521a_test.txt may0521a_test2.txt > may0521a_diff2.txt

# MANSTART in APR0323A
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2023/APR0323/oflsa -out apr0323a_test2
diff apr0323a_test.txt apr0323a_test2.txt > apr0323a_diff2.txt

# MANSTART in APR1723A
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2023/APR1723/oflsa -out apr1723a_test2
diff apr1723a_test.txt apr1723a_test2.txt > apr1723a_diff2.txt

# JUN1923 week big dither
/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2023/JUN1923/oflsa -out jun1923a_test2
diff jun1923a_test.txt jun1923a_test2.txt > jun1923a_diff2.txt

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.