search

growth-astro / growth-too-marshal

GROWTH Target of Opportunity Marshal
MIT License
13 stars 12 forks source link

Add mode_num to too_manual #139

Closed ebellm closed 3 years ago

ebellm commented 3 years ago

Does this pull request make any changes to the database? No.

Addresses #138

lpsinger commented 3 years ago

Instead of a "mode number" could we have textual descriptions of what the modes are?

ebellm commented 3 years ago

I agree that would be a better user experience and less prone to error--unfortunately the complete set of mode numbers is not yet enumerated by the camera team, as the mode number encodes a complex set of configuration parameters that they plan to test on-sky. I don't think we want to have to redeploy the marshal every time they tweak a parameter value in the readout electronics, so I suggest we stick with the box for now.

lpsinger commented 3 years ago

I don't think we want to have to redeploy the marshal every time they tweak a parameter value in the readout electronics

Why not? I think redeploying the marshal is entirely reasonable and expected when the values of the camera mode enumeration have changed.

ebellm commented 3 years ago

The camera team can and I expect will deploy new modes during testing within the night--if we have to redeploy the marshal to reflect those changes it will severely hinder testing velocity.

ebellm commented 3 years ago

(I'll add that the team is pushing to test on-sky tonight, so if we are able to get this through today it would be helpful.)

lpsinger commented 3 years ago

The camera team can and I expect will deploy new modes during testing within the night--if we have to redeploy the marshal to reflect those changes it will severely hinder testing velocity.

In that case, how would we even know which mode number to request?

ebellm commented 3 years ago

The scientist (@igorandreoni in this most immediate case) will be in close communication with the camera team during the testing period.

lpsinger commented 3 years ago

The CI pipeline is failing and actually dumping core. I have never seen that before! But it's clearly unrelated to this PR, so I'll merge now. We'll need to fix the CI pipeline and the Docker Hub builds in order to deploy, though.