sot / kadi

Chandra commands and events
https://sot.github.io/kadi
BSD 3-Clause "New" or "Revised" License
5 stars 3 forks source link

Update code and tests for use with ska_sun accurate position #304

Closed taldcroft closed 7 months ago

taldcroft commented 7 months ago

Description

This PR takes advantage of the associated PR's below (higher precision Sun position by default) to achieve improved accuracy of the off_nom_roll state. This remove those violations from the kadi validation page and allows a tighter check tolerance.

Despite 7 files being changed, there is only one substantive update in states.py, and even this one is not changing the behavior at all just doing the pitch/off_nom_roll calculation in a more efficient way.

The rest of the changes are just code tidying or adding a new development script for evaluating state speed.

Closes #270

Required dependent PRs

These are required to get the performance/accuracy gains and pass tests.

Interface impacts

Testing

Unit tests

kadi/commands/tests/test_commands.py ........................................................................... [ 34%] kadi/commands/tests/test_states.py ......................x.............................................x.............. [ 72%] ......... [ 76%] kadi/commands/tests/test_validate.py .................... [ 85%] kadi/tests/test_events.py .......... [ 89%] kadi/tests/test_occweb.py ...................... [100%]

========================================= 217 passed, 2 xfailed in 94.37s (0:01:34) ==========================================

Independent check of unit tests by Jean
- [x] Linux

### Functional tests
<!-- Describe and document results of any functional tests, otherwise leave the text below -->
#### Branch

(ska3-perf) ➜ validate git:(improve-off-nom-roll-state-accuracy) python performance_states.py
kadi.version='7.7.2.dev15+ge201f44' get_states took 3.4 sec 2nd get_states took 3.1 sec

#### Flight

(ska3) ➜ validate git:(improve-off-nom-roll-state-accuracy) python performance_states.py kadi.version='7.6.0' get_states took 6.2 sec 2nd get_states took 5.9 sec

jeanconn commented 7 months ago

I don't see where this PR does "Improve off-nominal roll state accuracy".

taldcroft commented 7 months ago

I don't see where this PR does "Improve off-nominal roll state accuracy".

Fair enough. I have updated the title and also the description:

This PR takes advantage of the associated PR's below (higher precision Sun position by default) to achieve improved accuracy of the off_nom_roll state. This remove those violations from the kadi validation page and allows a tighter check tolerance.

jeanconn commented 7 months ago

Do you have a conftest.py in kadi/commands/tests/ in your local repo that isn't commited to version control @taldcroft ?

taldcroft commented 7 months ago

Do you have a conftest.py in kadi/commands/tests

Yes, fixed.

taldcroft commented 7 months ago

Hum, that's interesting about the notebook. These are plotly figures, so apparently that doesn't get saved in the notebook or doesn't render on GitHub. Good to remember.