simonsobs / scheduler

Scheduler for SO
0 stars 1 forks source link

Safety checker script #102

Closed ktcrowley closed 1 month ago

ktcrowley commented 1 month ago

I've made a first pass at defining a class that can parse scheduler output .py files and determine if the specified moves, at the specified times, are actually sun-safe according to socs.agents.acu.avoidance module, specifically the command check_trajectory.

I'm still working to confirm this will catch all of the No sun-safe moves! or That move violates sun safety errors that have been crashing schedules on SATps in the past couple of months. The most recent such failure is caught (schedule beginning 20240916), but at least 1 other (schedule beginning 20240903) seems to pass -- so check_trajectory may not be the full story.

guanyilun commented 1 month ago

this looks great, thanks for working on this! Thinking about code organization, I wonder if it's better to manage this script under scheduler-scripts under sat/, where you will also have access to the config file and is more SAT specific?

kmharrington commented 1 month ago

I was asking to put in here. Thinking about it from the perspective of "scheduler should be able to check it's work," I think the final form of this script is actually some sort of function/checker inside this repo. So, the next step is integrating this script better into the scheduler software as a whole. In fact I know a few very quick changes I can make to this to generalize between all the SATs that I will be trying to make sometime this week.

But for now, it's a tool that I really wanted available while the final form is still on a farther off to-do list.

mhasself commented 1 month ago

(Seems like this is far from its final form so I'm making it a draft.)

guanyilun commented 1 month ago

I was asking to put in here. Thinking about it from the perspective of "scheduler should be able to check it's work," I think the final form of this script is actually some sort of function/checker inside this repo. So, the next step is integrating this script better into the scheduler software as a whole. In fact I know a few very quick changes I can make to this to generalize between all the SATs that I will be trying to make sometime this week.

I see, I agree it's important to have this kind of checks at multiple level, so I support having this script here! I was mostly just pointing out that this repo currently contains no actual driver script. It'll be better if this check can be more integrated in the library level, e.g., can it be added as a function of the SATPolicy class and called as part of the cmd2txt step to check the output text?

kmharrington commented 1 month ago

Not actually that far from a more final form. I moved this into the module but left it executable on existing schedules. Now it automatically pulls up the correct platform information and can be run when all the commands are held in memory.

Matching update for scheduler scripts. https://github.com/simonsobs/scheduler-scripts/pull/7

kmharrington commented 1 month ago

@guanyilun @mhasself Could we get a review for this? We've already had this branch save errors on satp1, I'd like to get in merged in