sot / starcheck

BSD 3-Clause "New" or "Revised" License
3 stars 0 forks source link

Add up NMM time before NPNT states to calculate maneuver equiv angle #432

Closed jeanconn closed 8 months ago

jeanconn commented 10 months ago

Description

For each NPNT state, add up the time in NMM before that state to calculate a maneuver angle "equivalent". This is used in the proseco reconstructed acquisition catalog for those checks and it is printed if it differs by more than 5 degrees from the maneuver angle of the actual maneuver to the NPNT state.

We retired this idea, but with the segmented maneuvers, I find the calculated maneuver angle output reassuring and this code seems overall benign.

This code also moves the ir_zone code into a new file (not 100% sold on the name of that file) with this new angle calculation code.

And while in there, updated the report output for the high ir zone checker to print what it is doing in minutes.

Fixes #435

Interface impacts

Testing

Unit tests

Independent check of unit tests by @taldcroft

Functional tests

Added some unit tests of the parts of the new code.

Regression test weeks in

https://icxc.cfa.harvard.edu/aspect/test_review_outputs/starcheck/starcheck-pr432/

I ran all weeks back to 2020:001 to look for anomalous behavior in the diffs, but then reduced the set to the recent two-week load and schedules that show new critical warnings with this PR. The output in there is the product of this script, where the PR code (from sandbox_starcheck) was run from a ska3-masters on fido.

touch combined_diff.txt

/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2023/DEC2523/oflsb -out DEC2523b_test
starcheck -dir /data/mpcrit1/mplogs/2023/DEC2523/oflsb -out DEC2523b_master
/proj/sot/ska/bin/diff2html DEC2523b_master.txt DEC2523b_test.txt > DEC2523b_diff.html
echo ---------------------------------------- >> combined_diff.txt
echo "week = DEC2523b" >> combined_diff.txt
diff -u DEC2523b_master.txt DEC2523b_test.txt >> combined_diff.txt

/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2023/MAY2923/oflsa -out MAY2923a_test
starcheck -dir /data/mpcrit1/mplogs/2023/MAY2923/oflsa -out MAY2923a_master
/proj/sot/ska/bin/diff2html MAY2923a_master.txt MAY2923a_test.txt > MAY2923a_diff.html
echo ---------------------------------------- >> combined_diff.txt
echo "week = MAY2923a" >> combined_diff.txt
diff -u MAY2923a_master.txt MAY2923a_test.txt >> combined_diff.txt

/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2023/APR0323/oflsa -out APR0323a_test
starcheck -dir /data/mpcrit1/mplogs/2023/APR0323/oflsa -out APR0323a_master
/proj/sot/ska/bin/diff2html APR0323a_master.txt APR0323a_test.txt > APR0323a_diff.html
echo ---------------------------------------- >> combined_diff.txt
echo "week = APR0323a" >> combined_diff.txt
diff -u APR0323a_master.txt APR0323a_test.txt >> combined_diff.txt

/home/jeanconn/git/starcheck/sandbox_starcheck -dir /data/mpcrit1/mplogs/2022/MAY2322/oflsa -out MAY2322a_test
starcheck -dir /data/mpcrit1/mplogs/2022/MAY2322/oflsa -out MAY2322a_master
/proj/sot/ska/bin/diff2html MAY2322a_master.txt MAY2322a_test.txt > MAY2322a_diff.html
echo ---------------------------------------- >> combined_diff.txt
echo "week = MAY2322a" >> combined_diff.txt
diff -u MAY2322a_master.txt MAY2322a_test.txt >> combined_diff.txt

Review of the new CRITICAL warnings shows that they are all due to time newly "counted" in NMM that was not counted at the time of review:

Obsid 44657 has a non-zero (manual) MANSTART=000:00:10:50.600 . https://icxc.cfa.harvard.edu/aspect/test_review_outputs/starcheck/starcheck-pr432/APR0323a_test.html#obsid44657

Obsid 25816 has a non-zero MANSTART=000:00:12:05.706 https://icxc.cfa.harvard.edu/aspect/test_review_outputs/starcheck/starcheck-pr432/MAY2322a_test.html#obsid25816

Obsid 44464 has a non-zero MANSTART=000:00:07:18.574 https://icxc.cfa.harvard.edu/aspect/test_review_outputs/starcheck/starcheck-pr432/MAY2923a_test.html#obsid44464

These functional tests were run at 47a15af

jeanconn commented 8 months ago

The plan is to squash and merge this when I've updated the functional test outputs. I should also add an issue to skare3_tools referencing this PR as an example of a squash and merge for tracking.

jeanconn commented 8 months ago

I think you were more than done with this @taldcroft but I re-requested review just because there were some substantive changes.

jeanconn commented 8 months ago

Right now, this PR uses the angle-from-nmm-time for acq probability if more than 5 degrees different from maneuver angle and prints it in the maneuver info above the catalog but there's no warning. Sounds like there needs to be a warning of some kind if angle-from-nmm-time is smaller than maneuver angle. And it sounds like in that case the acq probability should be calculated with the larger angle.

taldcroft commented 8 months ago

I was hoping to avoid another revision that you're right.

jeanconn commented 8 months ago

I'm taking your "that" as a "but" in the previous comment and I put in some tiny changes to add a critical warning for the unexpected case and to change the "use the angle-from-nmm" code to only use that angle for probabilities if more than 5 degrees greater.

jeanconn commented 8 months ago

Great! I note I redid the unit and functional tests at 47a15af .