sot / chandra_cmd_states

Interface for creation and maintenance of the Chandra commanded states database.
https://sot.github.io/cmd_states
BSD 3-Clause "New" or "Revised" License
3 stars 1 forks source link

Special case WSPOW00000 (specifically turn off vid board) #31

Closed jeanconn closed 7 years ago

jeanconn commented 7 years ago

"WSPOW00000 turns off all FEPs and video boards" per @jzuhone . This PR special-cases that power command to turn the vid board off.

jeanconn commented 7 years ago

I hadn't made much progress with this because it is relatively low-priority and I wasn't sure about minimum testing. I've just called this code on a testing copy of the sqlite version of the cmds states database, and can confirm that the WSPOW00000 now appears to have the right effect on vid_board status in the cmd_states table:

Old

sqlite> select datestart, datestop, vid_board, power_cmd from cmd_states where datestart > '2017:250' and datestart < '2017:251';
2017:250:00:05:30.816|2017:250:02:41:49.025|1|XTZ0000005
2017:250:02:41:49.025|2017:250:02:42:54.685|1|XTZ0000005
2017:250:02:42:54.685|2017:250:02:42:55.710|1|AA00000000
2017:250:02:42:55.710|2017:250:02:43:05.960|1|AA00000000
2017:250:02:43:05.960|2017:250:02:52:10.816|1|WSPOW00000
2017:250:02:52:10.816|2017:250:05:38:50.816|1|WSPOW00000
2017:250:05:38:50.816|2017:250:08:25:30.816|1|WSPOW00000
2017:250:08:25:30.816|2017:250:11:12:10.816|1|WSPOW00000
2017:250:11:12:10.816|2017:250:13:58:50.816|1|WSPOW00000
2017:250:13:58:50.816|2017:250:16:45:30.816|1|WSPOW00000
2017:250:16:45:30.816|2017:250:18:32:02.000|1|WSPOW00000
2017:250:18:32:02.000|2017:250:19:32:10.816|1|WSPOW00000
2017:250:19:32:10.816|2017:250:22:18:50.816|1|WSPOW00000
2017:250:22:18:50.816|2017:251:01:04:33.509|1|WSPOW00000

New

sqlite>  select datestart, datestop, vid_board, power_cmd from cmd_states where datestart > '2017:250' and datestart < '2017:251';
2017:250:00:05:30.816|2017:250:02:41:49.025|1|XTZ0000005
2017:250:02:41:49.025|2017:250:02:42:54.685|1|XTZ0000005
2017:250:02:42:54.685|2017:250:02:42:55.710|1|AA00000000
2017:250:02:42:55.710|2017:250:02:43:05.960|1|AA00000000
2017:250:02:43:05.960|2017:250:02:52:10.816|0|WSPOW00000
2017:250:02:52:10.816|2017:250:05:38:50.816|0|WSPOW00000
2017:250:05:38:50.816|2017:250:08:25:30.816|0|WSPOW00000
2017:250:08:25:30.816|2017:250:11:12:10.816|0|WSPOW00000
2017:250:11:12:10.816|2017:250:13:58:50.816|0|WSPOW00000
2017:250:13:58:50.816|2017:250:16:45:30.816|0|WSPOW00000
2017:250:16:45:30.816|2017:250:18:32:02.000|0|WSPOW00000
2017:250:18:32:02.000|2017:250:19:32:10.816|0|WSPOW00000
2017:250:19:32:10.816|2017:250:22:18:50.816|0|WSPOW00000
2017:250:22:18:50.816|2017:251:01:04:33.509|0|WSPOW00000

What else do we need for testing?

jeanconn commented 7 years ago

For the record, to run the code on-the-side on a local copy of the sqlite version of the table I did:

./update_cmd_states.py --dbi sqlite --server cmd_states.db3 --h5file cmd_states.h5 --datestart '2017:240'
jeanconn commented 7 years ago

OK, modified and re-verified. I just looked at text diffs of the cmd_states since 2017:240. Looks as-expected to me. No new states, just vid_board = 0 for all the states with WSPOW00000.

old_states.txt

new_states.txt

jeanconn commented 7 years ago

I've called this release 3.12.

jeanconn commented 7 years ago

@jzuhone These changes are merged, but because we're hitting a routine called by the LR tools was wondering about ACIS running a test before we bring to LR for approval. Thoughts?

jzuhone commented 7 years ago

@jeanconn I think this can be arranged--I'll try to set something up today or tomorrow.

jzuhone commented 7 years ago

Hi @jeanconn,

I was able to run some thermal models with these changes. Since the video boards are in the DEA, it's the only model that's really affected by this (and even then probably not really that much). Here's before and after plots with the change to the video boards during our recent radiation shutdowns:

Before: before

After: after

However, the main thing is that I added a new ACIS power state coefficient pow_0000 which is modeling these in a more fine-grained way than before, which is probably the main reason for the improvement.

jeanconn commented 7 years ago

Hi @jzuhone . Thanks! I'm a little confused by your "the main thing is that I added a new ACIS power state coefficient pow_0000" statement, though. Are both runs of the model you are showing are using the same version of dea_check with the new coefficient?

jzuhone commented 7 years ago

@jeanconn no, only the new model has the updated power coefficient. In short, I don't really think this affects our model fitting much after trying it (at least not now), but it's good to have consistency in the database.

jeanconn commented 7 years ago

So I'm still a bit confused?. Wouldn't the plan be to run the current/new dea code once with the bad power states and once with this PR's power states and compare the two?

jzuhone commented 7 years ago

that's what I did originally (without adding the extra coefficient), but it didn't really make any difference. The only thing that made a difference was adding a new coefficient. Which, of course, is a separate issue altogether. Sorry for the confusion.

jeanconn commented 7 years ago

Ah. That's where I was confused. I thought the new coefficient was already in current dea_check (but hadn't been at the time of the last radiation shutdown). But you were trying to tell me it was in a dev copy you were using to actually see a change. Gotcha! That's fine then. If all we are showing by running the dea_check on the side with this PR is that the tool runs without errors and the model output isn't any worse (and that with a new coefficient it can actually be better) I'm satisfied that counts as comprehensive testing.