simonsobs / sodetlib

Tools for performing core instrument testing, quality control, and analysis tasks.
BSD 2-Clause "Simplified" License
5 stars 0 forks source link

Bias Wave #412

Closed msilvafe closed 6 months ago

msilvafe commented 7 months ago

This is a draft PR for now...still lots to go but can start code level discussion here to move some of it off of slack.

The intention of this PR is to implement a new "bias wave" module that calculates equivalent parameters as the bias_step operation but using small sine waves on the bias line which can be processed using fourier analysis which is a bit cleaner in the presence of large halfwave plate synchronous signal.

msilvafe commented 7 months ago

Ok, I think this is ready to test. It currently can take data at multiple frequencies but only runs the analysis on the lowest frequency to calculate the DC parameters. We can add in analysis functions later for calculating time constants with multiple frequencies (there's some inline comments with placeholders for those things to go).

This should probably come with an accompanying PR in socs to add an agent task but that can come after testing. I'm going to convert from draft to full PR so you all can start reviewing but I we will wait until we have taken some test data on SATP1 or SATP3 before merging.

bkelle commented 6 months ago

Spent some time looking through this this morning. I think it looks good and I agree its ready to test as soon as we have a telescope we can try to run it on. That being said i'm not a total expert when it comes to sodetlib code writing. But I think this captures what Yuhan and I were doing in notebooks. Do I need to approve this commit before being able to test it, or should we test it and then approve after that?

msilvafe commented 6 months ago

Spent some time looking through this this morning. I think it looks good and I agree its ready to test as soon as we have a telescope we can try to run it on. That being said i'm not a total expert when it comes to sodetlib code writing. But I think this captures what Yuhan and I were doing in notebooks. Do I need to approve this commit before being able to test it, or should we test it and then approve after that?

Please test on this branch, I'm not confident that the data taking won't just fail because I have a missed colon at this point...(I mostly don't think that's true bc I did run a linter on this but I think you get my point). So checkout this branch on the server and test the data taking and analysis fn's and confirm that everything looks good. Post whatever plots/etc are useful from your testing here in the review to show that you've tested that it works and that can serve as your approval to merge.

bkelle commented 6 months ago

Sounds good Max. We will do that. Hopefully we can work it in to the SATp3 schedule when observing again, I think things have been down for a few days? In any case I'll interface with Yuhan, Sam, and folks at the site to test in this branch. thanks!

msilvafe commented 6 months ago

Sounds good Max. We will do that. Hopefully we can work it in to the SATp3 schedule when observing again, I think things have been down for a few days? In any case I'll interface with Yuhan, Sam, and folks at the site to test in this branch. thanks!

Ok, sounds good! Let me know when you get a chance to take a stab at it. I can try to be available to help debug any issues that arise.