search

simonsobs / scheduler

Scheduler for SO
0 stars 1 forks source link

delete socs dependence for sun safety checker #123

Open ykyohei opened 4 weeks ago

ykyohei commented 4 weeks ago

This should be important for internal consistency.

kmharrington commented 4 weeks ago

One of the reasons I'd kept that with the socs version is that the sun safety checker should use the most up to date setup for how the agent on the telescope will determine if the positions are sun safe or not (since that agent is what throws the sun safety errors). If the telescope OCS agent evolves but that third-party module doesn't then the scheduler will start spitting out schedules that it will then check and incorrectly say they're sun safe.

Honestly, since socs is installable via pip I think I'd rather remove third party and just use the actual module.

@mhasself thoughts on this?

mhasself commented 3 weeks ago

Ya, tough call on this. socs has a lot of dependencies, which is annoying. Although the import from socs helps keep the avoidance code ~current, it doesn't keep the avoidance parameters current, because those are config params passed in through ocs default.yaml and/or the ACU config file. So they'd need to be updated here somehow, separately, too.

My inclination is to keep this scheduler file as a close copy of the socs version, and not add a dependency on socs.

That being said: the current scheduler version of avoidance.py includes many many changes relative to the socs one, which makes comparing with the socs version relatively difficult. I think the "third_party" version should contain minimal changes / additions -- only what's necessary for integration. Any additional supporting code should be added in a separate file.