sot / kadi

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

Factor out `get_ofp_states` and `get_telem_values` for cheta #249 improvements #285

Closed taldcroft closed 1 year ago

taldcroft commented 1 year ago

Description

This PR is paired with https://github.com/sot/cheta/pull/249 to factor out the get_ofp_states and get_telem_values functions into cheta where they more logically belong.

Interface impacts

get_ofp_states and get_telem_values are no longer available in kadi.utils. Both of these are currently used only internally within cheta and kadi in SOT repos on GitHub. Any other usages in non-versioned tools can just be fixed.

The API for both of these were changed as well, hence it would not make sense to include a reference in kadi.utils (from cheta) for back-compatibility.

Testing

Unit tests

Independent check of unit tests by [REVIEWER NAME]

Functional tests

Validation script was run from inside the repo for the branch outputs and outside the repo for release. The cheta repository was checked out with https://github.com/sot/cheta/pull/249.

In both cases below I did a side-by-side visual / by-eye text comparison of the outputs from this branch and ska3 2023.1 respectively. No diffs were seen.

Run with default arguments covering last 14 days (uses MAUDE)

env PYTHONPATH=/Users/aldcroft/git/cheta python -m kadi.scripts.validate_states --out-dir current-branch python -m kadi.scripts.validate_states --out-dir current-release

Run with arguments covering 2022:283 safe mode (no MAUDE)

env PYTHONPATH=/Users/aldcroft/git/cheta python -m kadi.scripts.validate_states --out-dir safe-2022-branch --stop=2022:297 --days=5 python -m kadi.scripts.validate_states --out-dir safe-2022-release --stop 2022:297 --days=5

taldcroft commented 1 year ago

@javierggt - thanks for the review. I agree we need to wait on cheta# 249. About the diffs, for the record here it is and I'll look into it. I also note that plotly seems to be doing something weird with the negative sign. This occurs in all the versions so it is not a regression here but still should be fixed.

TA

image

JG

image
taldcroft commented 1 year ago

@javierggt - about the difference in SIM-Z between our respective runs, I note that the additional bad data point in your plot is within a couple seconds of a time when the OFP is doing something weird at 2022:294:16:37:27.347 CONLOFP=STUP.

When I directly look at telemetry from cheta this glitch does not seem present:

In [13]: from cheta import fetch_eng

In [14]: dat = fetch_eng.Msid("3tscpos", "2022-10-21", "2022-10-22")

In [15]: set(dat.vals)
Out[15]: {-99616.0}

Do you get the same?

taldcroft commented 1 year ago

About the malformed minus sign, this is discussed at https://stackoverflow.com/questions/75234217/gibberish-malformed-negative-y-axis-values-in-plotly-charts-in-python, but with no resolution.

taldcroft commented 1 year ago

cheta 249 is merged now, so I'm merging this one.