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

jeanconn · 2023-10-19T15:22:37Z

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

Mac at 47a15af

Independent check of unit tests by @taldcroft

Mac

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

starcheck/state_checks.py

starcheck/src/lib/Ska/Starcheck/Obsid.pm

taldcroft

Looks generally good!

~~I wonder if we want to add the effective maneuver angle to the starcheck output, or maybe only if it is different from the actual by more than a couple degrees?~~ [EDIT, already done! But maybe with a tighter tolerance.]

There is also the opportunity to cross-check the angle in the proseco pickle vs the effective man angle.

starcheck/state_checks.py

taldcroft

This all looks good pending the update to the high-IR zone text. Thanks!

jeanconn · 2023-12-20T17:49:19Z

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 · 2023-12-21T17:31:47Z

starcheck/src/lib/Ska/Starcheck/Obsid.pm

+    # To satisfy those goals, use the derived-from-NMM-time angle if it is more than 5 degrees
+    # different from the actual maneuver angle.  Otherwise use the actual maneuver angle.
+    # This also lines up with what is printed in the starcheck maneuver output.
+    my $man_angle = (abs($targ_cmd->{man_angle_calc} - $targ_cmd->{angle}) > 5)


@taldcroft Does this work for you? It is a substantive update since your approval. I'm rerunning functional review with it in place. To be fully conservative I suppose the other option would be to use the angle-from-nmm-time only if 5 or more degrees greater than the maneuver angle (instead of just different).

If the angle-from-nmm-time is more than 5 degrees SMALLER than the maneuver angle, that would indicate a bug in the code somewhere. In that case there should be a critical warning and the review should use the maneuver angle.

Nice comment BTW!!

Also adds processing warning if weird continuity initial pcad_mode state.

jeanconn · 2023-12-26T14:56:51Z

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

taldcroft · 2023-12-27T11:34:08Z

starcheck/src/lib/Ska/Starcheck/Obsid.pm

+    # To satisfy those goals, use the derived-from-NMM-time angle if it is more than 5 degrees
+    # different from the actual maneuver angle.  Otherwise use the actual maneuver angle.
+    # This also lines up with what is printed in the starcheck maneuver output.
+    my $man_angle = (abs($targ_cmd->{man_angle_calc} - $targ_cmd->{angle}) > 5)


If the angle-from-nmm-time is more than 5 degrees SMALLER than the maneuver angle, that would indicate a bug in the code somewhere. In that case there should be a critical warning and the review should use the maneuver angle.

taldcroft · 2023-12-27T11:34:55Z

starcheck/src/lib/Ska/Starcheck/Obsid.pm

+    # To satisfy those goals, use the derived-from-NMM-time angle if it is more than 5 degrees
+    # different from the actual maneuver angle.  Otherwise use the actual maneuver angle.
+    # This also lines up with what is printed in the starcheck maneuver output.
+    my $man_angle = (abs($targ_cmd->{man_angle_calc} - $targ_cmd->{angle}) > 5)


Nice comment BTW!!

taldcroft · 2023-12-27T11:35:41Z

starcheck/src/lib/Ska/Starcheck/Obsid.pm

@@ -3038,7 +3064,7 @@ sub proseco_args {
            0 + $targ_cmd->{q3},
            0 + $targ_cmd->{q4}
        ],
-        man_angle => 0 + $targ_cmd->{angle},
+        man_angle => 0 + $man_angle,


What a silly language with the 0 + something.

Some of this could probably go away -- I played some games here at one point to get floats over to Python but we could probably just be explicit on the Python side especially now that Inline::Python is gone.

taldcroft

But thinking some more, the existing warning that there is a > 5 degree difference should give enough warning to the reviewer and we will know to treat the SMALLER case as critical. So I'm approving now with the existing testing.

jeanconn · 2023-12-27T13:52:06Z

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 · 2023-12-27T14:43:05Z

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

jeanconn · 2023-12-27T15:22:11Z

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.

taldcroft

The latest changes look good.

jeanconn · 2023-12-28T14:36:29Z

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

jeanconn force-pushed the add-up-nmm branch from 4505b1c to 238ac63 Compare November 28, 2023 18:36

jeanconn commented Nov 28, 2023

View reviewed changes

starcheck/state_checks.py Outdated Show resolved Hide resolved

jeanconn commented Nov 28, 2023

View reviewed changes

starcheck/src/lib/Ska/Starcheck/Obsid.pm Outdated Show resolved Hide resolved

jeanconn marked this pull request as ready for review November 28, 2023 19:03

jeanconn changed the title ~~WIP: Add up nmm~~ Add up nmm time before npnt states to calculate maneuver equiv angle Nov 28, 2023

jeanconn requested a review from taldcroft November 28, 2023 19:04

taldcroft changed the title ~~Add up nmm time before npnt states to calculate maneuver equiv angle~~ Add up NMM time before NPNT states to calculate maneuver equiv angle Nov 29, 2023

taldcroft reviewed Nov 29, 2023

View reviewed changes

starcheck/state_checks.py Outdated Show resolved Hide resolved

taldcroft approved these changes Dec 19, 2023

View reviewed changes

jeanconn commented Dec 21, 2023

View reviewed changes

jeanconn added 16 commits December 22, 2023 00:53

Calculate maneuver angle from NMAN duration

7efb6c1

Update comment for calculated angles

6c36fd7

Add explicit unit for time delta

f0ecf2d

Reorganize and add some unit tests

d84c3ee

Respond to Tom's first-round/simple comments

9c614e0

Simplify NMM calc by assuming NPNT start

f964ec6

Also adds processing warning if weird continuity initial pcad_mode state.

Reduce tolerance of angle mismatch to 5 deg

16dcd34

Set to work with warning text from get_obs_man_angle

836fbe4

More reconfiguring and doc updates

974e029

Update tests for new state check handling

7cbd28f

Fixup formatting

aa51408

Rename continuity check, misc doc fix

505f6d1

Change return type of continuity check function

ef10640

Update docstrings

e063417

Update IR Zone text per Tom

5b39a13

Use maneuver angle instead of time-calc man angle unless different

06d5af1

jeanconn force-pushed the add-up-nmm branch from 9086da3 to 06d5af1 Compare December 22, 2023 14:32

jeanconn requested a review from taldcroft December 26, 2023 14:56

taldcroft reviewed Dec 27, 2023

View reviewed changes

taldcroft approved these changes Dec 27, 2023

View reviewed changes

jeanconn added 2 commits December 27, 2023 10:15

Use calculated maneuver angle if more than 5 deg larger

2bb9d9a

Print a warning if calc angle more than 5 deg smaller

47a15af

taldcroft approved these changes Dec 28, 2023

View reviewed changes

jeanconn merged commit 20b7c0a into master Dec 28, 2023

jeanconn mentioned this pull request Dec 28, 2023

Handle squash and merge in history for releases sot/skare3_tools#105

Closed

javierggt mentioned this pull request Jan 11, 2024

2023.10 sot/skare3#1242

Merged

javierggt mentioned this pull request Feb 6, 2024

2023.11 sot/skare3#1246

Merged

jeanconn deleted the add-up-nmm branch September 25, 2024 15:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

jeanconn commented Oct 19, 2023 •

edited

Loading

taldcroft left a comment •

edited

Loading

taldcroft left a comment

jeanconn commented Dec 20, 2023

jeanconn Dec 21, 2023

taldcroft Dec 27, 2023

taldcroft Dec 27, 2023

jeanconn commented Dec 26, 2023

taldcroft Dec 27, 2023

taldcroft Dec 27, 2023

taldcroft Dec 27, 2023

jeanconn Dec 27, 2023

taldcroft left a comment

jeanconn commented Dec 27, 2023

taldcroft commented Dec 27, 2023

jeanconn commented Dec 27, 2023

taldcroft left a comment

jeanconn commented Dec 28, 2023

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

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

Conversation

jeanconn commented Oct 19, 2023 • edited Loading

Description

Interface impacts

Testing

Unit tests

Functional tests

taldcroft left a comment • edited Loading

Choose a reason for hiding this comment

taldcroft left a comment

Choose a reason for hiding this comment

jeanconn commented Dec 20, 2023

jeanconn Dec 21, 2023

Choose a reason for hiding this comment

taldcroft Dec 27, 2023

Choose a reason for hiding this comment

taldcroft Dec 27, 2023

Choose a reason for hiding this comment

jeanconn commented Dec 26, 2023

taldcroft Dec 27, 2023

Choose a reason for hiding this comment

taldcroft Dec 27, 2023

Choose a reason for hiding this comment

taldcroft Dec 27, 2023

Choose a reason for hiding this comment

jeanconn Dec 27, 2023

Choose a reason for hiding this comment

taldcroft left a comment

Choose a reason for hiding this comment

jeanconn commented Dec 27, 2023

taldcroft commented Dec 27, 2023

jeanconn commented Dec 27, 2023

taldcroft left a comment

Choose a reason for hiding this comment

jeanconn commented Dec 28, 2023

jeanconn commented Oct 19, 2023 •

edited

Loading

taldcroft left a comment •

edited

Loading