Skip to content

Change "creep-away" definition from 5deg to 3deg#455

Merged
jeanconn merged 5 commits intomasterfrom
creep-three
Aug 22, 2025
Merged

Change "creep-away" definition from 5deg to 3deg#455
jeanconn merged 5 commits intomasterfrom
creep-three

Conversation

@jeanconn
Copy link
Copy Markdown
Contributor

@jeanconn jeanconn commented Jul 29, 2025

Description

Set creep-away criterion to man_angle_next < 3.0 deg (a change from 5.0 deg).

This is consistent with discussion at SS&AWG on 2025x07x30 to use a 3 degree maneuver angle as the maximum allowed maneuver angle for "creep-away".

https://occweb.cfa.harvard.edu/twiki/bin/view/Aspect/StarWorkingGroupMeeting2025x07x30

Interface impacts

An update to the code to calculate maneuver angle from time reduces most estimated maneuver angles by ~0.77 deg.

Testing

Unit tests

  • Mac
(ska3-latest) flame:starcheck jean$ pytest
====================================================== test session starts ======================================================
platform darwin -- Python 3.12.8, pytest-8.3.4, pluggy-1.5.0
rootdir: /Users/jean/git
configfile: pytest.ini
plugins: socket-0.7.0, anyio-4.7.0, timeout-2.3.1
collected 14 items                                                                                                              

starcheck/tests/test_state_checks.py .............                                                                        [ 92%]
starcheck/tests/test_utils.py .                                                                                           [100%]

====================================================== 14 passed in 4.06s =======================================================
(ska3-latest) flame:starcheck jean$ git rev-parse HEAD
89320b4efe1a991706612562e31e48e91b7aa669

Independent check of unit tests by [REVIEWER NAME]

  • [PLATFORM]:

Functional tests

With regard to the maneuver angle calculation from time and adding a point at 3 degrees to the curve - it looks like the angle vs time curve has already basically flattened out by 3 degrees and it is very close to linear from 2 degrees to 5 degrees, so adding the point doesn't make that much difference.

image

With the original angles
angles = [0, 5, 10, 15, 20, 25, 35, 50, 100, 150, 180]

np.interp(3, old_table['angle'], old_table['duration'])
374.6850661223459

With the new maneuver angles:
angles = [0, 3, 5, 10, 15, 20, 25, 35, 50, 100, 150, 180]
np.interp(3, new_table['angle'], new_table['duration'])
375.59478586422125

So the original angles and interpolation would have underestimated the time of a 3 degree maneuver (as directly calculated by chandra_maneuver) by 0.9 seconds. Updating the list of angles is not costly in this case, but not a big effect.

With regard to functional testing of the starcheck code for these warnings, I edited two schedules to disable dither. Schedule JUL3123B has a 3.2 degree maneuver in it, so I disabled dither before that observation and confirm that the starcheck output now shows that the dither disabled observation does not have an acceptable creep-away.

https://icxc.cfa.harvard.edu/aspect/test_review_outputs/starcheck/starcheck-pr455/diff.txt

I also edited the dither of an observation in JUN3025A that had a 2.73 maneuver away, and it correctly shows no change in behavior (as 2.73 < 3 and still < 5).

https://icxc.cfa.harvard.edu/aspect/test_review_outputs/starcheck/starcheck-pr455/jun3025a_dither_test.html#obsid42309

https://icxc.cfa.harvard.edu/aspect/test_review_outputs/starcheck/starcheck-pr455/nodiff.txt

The code for this is very isolated, so I did not run a large regression set, just the last two weeks. I also included JAN0325A as it included no-dither observations.

https://icxc.cfa.harvard.edu/aspect/test_review_outputs/starcheck/starcheck-pr455/combined_diff.txt

@jeanconn
Copy link
Copy Markdown
Contributor Author

@taldcroft it looks to me like there's another sparkles vs starcheck asymmetry because starcheck isn't handling the "low guide count but not too low (3.5 to 4)" case as also requiring creep-away (or being allowed if creep-away used). Do we care? If so I could probably stick that in this PR too.

@jeanconn
Copy link
Copy Markdown
Contributor Author

We decided in-person not to worry about the 3.5 to 4.0 guide count case in this PR.

@jeanconn jeanconn marked this pull request as ready for review August 13, 2025 19:20
@jeanconn jeanconn requested review from javierggt and taldcroft and removed request for javierggt and taldcroft August 13, 2025 19:20
@jeanconn jeanconn marked this pull request as draft August 13, 2025 19:24
@jeanconn jeanconn marked this pull request as ready for review August 19, 2025 17:29
@jeanconn jeanconn requested a review from Copilot August 19, 2025 18:19

This comment was marked as resolved.

@jeanconn
Copy link
Copy Markdown
Contributor Author

Yeah, in conversation this is too ugly so let's figure out why the NMM->angle is not a good match for some of the small angle cases that failed my initial test and either fix that or understand it enough to cut the overcomplications in this code.

@jeanconn
Copy link
Copy Markdown
Contributor Author

I subtracted the extra 10.25 seconds of NMM "hold time" between AONMMODE and AOMANUVR and that looks to have lined up the NMM->angle calculation a bit more in my limited test cases, such that we can just use the NMM->angle calc for this.

I had an earlier draft that also tried to simplify by pushing the calculated maneuver angle into the MP_TARGQUAT perl data structure to avoid the duplication of having get_obs_man_angle_next and get_obs_man_angle but maybe the duplication is helpful for now.

@jeanconn
Copy link
Copy Markdown
Contributor Author

I reran the starcheck functional testing of the two schedules and the regression testing at 0b790cc

Comment thread starcheck/state_checks.py
@jeanconn
Copy link
Copy Markdown
Contributor Author

And I should have recorded that there's a tiny interface impact here, in that the estimated angles from NMM time are now reduced by that 10.25s, so in regression any time that starcheck would have printed the NMM->angle value, now the angle is a little bit smaller (and more accurate!).

This is visible in those combined diffs in regression:

-  MANVR: Calculated angle from NMM time = 156.02 deg
+  MANVR: Calculated angle from NMM time = 155.25 deg

@jeanconn jeanconn merged commit 44d691e into master Aug 22, 2025
2 checks passed
@jeanconn jeanconn deleted the creep-three branch August 22, 2025 14:35
@javierggt javierggt mentioned this pull request Sep 22, 2025
@javierggt javierggt mentioned this pull request Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants