search

sot / cheta

Cheta Telemetry Archive
https://sot.github.io/eng_archive
BSD 3-Clause "New" or "Revised" License
3 stars 4 forks source link

Computed MSID for pitch/off-nominal roll + data gap + docs fixes #242

Closed taldcroft closed 1 year ago

taldcroft commented 1 year ago

Description

This is a new computed MSID to return the Sun pitch angle or off-nominal roll angle which is valid in NPNT, NMAN, NSUN, and Safe Mode. This new computed MSID also works with MAUDE as the back-end telemetry server (unlike e.g. dp_pitch/roll).

Current available MSIDs are limited in scope. For instance dp_pitch is only good for NPNT or NMAN when PCAD processing is maintaining AOATTQT1-4 accurately. Querying dp_pitch in other modes will return a value that is wrong.

The logic is:

Additional changes

A couple more things crept into this PR for convenience:

Interface impacts

Adds new MSIDs pitch_comp and roll_comp.

Testing

Unit tests

Independent check of unit tests by [REVIEWER NAME]

Functional tests

Used in https://github.com/sot/kadi/pull/261.

jeanconn commented 1 year ago

Needs a test. I note this needs https://github.com/sot/kadi/pull/261 to actually calculate computed pitch|roll but passes tests without that in path because get_ofp_states is never executed.

jeanconn commented 1 year ago

For this PR I'm a little surprised by these values:

dat = fetch.Msid('pitch_comp', '2022:294:16:00:00.000', '2022:294:17:00:00.000')
dat.plot()

pitch_comp

taldcroft commented 1 year ago

For this PR I'm a little surprised by these values:

Safe mode entry was at 2022:294:16:32:25.285, so that deviation is the first 5 minutes of safe mode. I'm guessing there is some part of the on-board CPE calculation that is not yet initialized.

From a practical perspective of thermal propagation this won't have any impact so I'd be inclined to not worry about this too much.

jeanconn commented 1 year ago

Right, I was just working from "always valid" in the PR title and it seemed like ~20 degrees off in pitch even for a short time was not the right answer. I was just looking at this independent from the kadi validation perspective and trying to figure out what confidence to have in the values.

taldcroft commented 1 year ago

Fair enough, I changed the PR title. :smile: For kadi validation this 5-minutes of invalid values gets blocked out because of a CONLOFP=STUP interval right on safe mode transition.

jeanconn commented 1 year ago

Also good for me to keep in mind that you are using the ofp states for the pitch|roll here but you have padded exclude intervals in the validation. I still think this needs at least a small test but wasn't sure what made sense to hit the code paths.

taldcroft commented 1 year ago

@javierggt - good point about docs. I took the opportunity to improve all the computed MSID docs. See: https://cxc.cfa.harvard.edu/mta/ASPECT/tmp/cheta_docs/pseudo_msids.html#computed-msids

As I was doing this I wasted a bunch of time being confused by the sphinx docs builds when the changes I made locally didn't get reflected in the rendered docs. I finally realized that all the doc references to Ska.engarchive were being taken from the installed Ska.engarchive not local. YET another problem of namespace packages.

With that motivation, I just decided to change every Ska.engarchive in the docs to cheta. This is obviously out of scope for this PR, but it's localized in one commit and making a separate PR would have just complicated life.

taldcroft commented 1 year ago

Note that in 3936f79, I created the regression values before doing the refactoring because I was worried about breaking the derived params DP_PITCH/ROLL_CSS code. But since it passes tests both before and after the refactor I feel confident in the change.