WIP: Improve pitch value for long dwells

[taldcroft]

Commanded states just knows about the pitch immediately following a maneuver and doesn't correctly handle the Sun moving during the dwell. In theory we could fix this in Chandra/cmd_states/update_cmd_states.py. In the final bit of get_states() there is a loop where it would be convenient to fix up the state pitch values based on the attitude and start / stop times. Instead of using pitch at the start, use the mean of pitch at the start / stop, or even the mean of 10 evenly spaced values:

    for state in states:
        state['tstart'] = DateTime(state['datestart']).secs
        state['tstop'] = DateTime(state['datestop']).secs
        state['pitch'] = get_mean_pitch(state)  # NEW

get_mean_pitch() presumable uses Ska.Sun.pitch(ra, dec, time).

Note that during maneuvers this averaging is already done, so the fixing should only be done for dwells longer than an hour.

cc: @jeanconn

[jeanconn]

Should we improve this one pitch value using the mean (as described above), or should we break up the states for long dwells to provide more pitch sampling, either breaking the states into fixed time chunks or new cmd_states when the pitch has changed by a certain amount?

[taldcroft]

I think the easiest is going to be fixed time chunks. I'm a little stumped in how this would be nicely implemented. Maybe you'll have an idea. :smile:

[jeanconn]

Maybe something like this? Needs cleanup but I think my proof-of-concept works.

[taldcroft]

This approach relies on not having any state-changing commands within a long dwell. In practice that is probably OK.

Overall I think what you've done will work. I wish there were a more elegant solution but I couldn't find one. I hope it won't be too terribly long before this gets superceded by dynamic cmd states.

[jeanconn]

I think it doesn't really rely on not having any state-changing commands within a long-dwell, because it is using the actual commands to find long gaps and add these pitch commands. If there were any state-changing commands, they would already ... be there. The only commands that are "generated" are the ones you've made in maneuvers, so I think that is the only place we need to worry about race conditions.

And I thought it was a relatively elegant solution. Things being what they are :-) ​

[taldcroft]

Finished review.

[taldcroft]

For the sake of discussion imagine that the loads enable and disable dither in turn every 1 ksec in NPM. My understanding of the code is that the the dither enable and dither disable transitions will not update the pitch value, but those commands will make every element of long_duration be False. This prevents any pitch update psuedo-commands from being generated. Am I missing the logic?

[jeanconn]

Good point! That is why my first thought was to use just evenly spaced clock-out commands over the duration but I was stymied by the maneuvers. I think we could either go with something like this PR with the flaw you note or or we could

• use evenly-spaced clock-out PITCH commands
• pull out the maneuver state generation into something similar to the make_pitch_cmds thing (make a command list) instead of doing that process in the state building

My thought was if we made maneuver-clock-out "commands" before the state-building loop (and sorted the commands by time) there'd be no problem with GET_PITCH cmds overlapping a maneuver in progress.

[jeanconn]

Actually, I suppose you could use evenly-spaced clock-out pitch commands and leave maneuvers as they are if you were just good about skipping a mock command in the state-building loop if the date of your last-processed command came after the current-command... Oy.

[taldcroft]

The elusive "more elegant solution" was something really simple and clean that was bulletproof. We haven't found this, but I am quite convinced that your current implementation will provide the intended benefit in practice. That's because none of the state-changing commands typically occur in the middle of long observations, and there is zero chance of having a string of state-changing commands in the middle of an observation where they occur at intervals less than 5 ksec.

So just fix the minor comments and you're done.

[jeanconn]

Closed in favor of #21