Skip to content

Stimulator analysis pipeline#1659

Merged
ykyohei merged 43 commits into
masterfrom
stimulator_analysis_pipeline
Jun 25, 2026
Merged

Stimulator analysis pipeline#1659
ykyohei merged 43 commits into
masterfrom
stimulator_analysis_pipeline

Conversation

@YudaiSeino

Copy link
Copy Markdown
Contributor

I wrote stimulator analysis code. Main functions are calc_gain() and calc_timeconstant(). calc_gain() calculates stimulator's signal amplitude. calc_timeconstant() calculates optical time constant and readout delay.

@ykyohei ykyohei left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's great to see the simulator pipeline in sotodlib!

Preliminary review comments. sotodlib follows PEP8, but whitespace is missing after commas in many places. I suggest running a linter and correcting them.

Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated

@ykyohei ykyohei left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! it's getting much better!
I guess I found few bugs, please check.

Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated
@YudaiSeino YudaiSeino requested a review from ykyohei June 24, 2026 21:44
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py
Comment thread sotodlib/stimulator/stimulator.py Outdated
Comment thread sotodlib/stimulator/stimulator.py
Comment thread sotodlib/stimulator/stimulator.py Outdated

@ykyohei ykyohei left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the changes, almost there!

overwrite=True is used in many places. I think these were convenient when developping pipeline but these are not really necessary. I'll let you decide but and it might be safer to remove.

@YudaiSeino YudaiSeino requested a review from ykyohei June 25, 2026 19:48
@ykyohei

ykyohei commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Looks good, thank you!

@ykyohei ykyohei merged commit d133f84 into master Jun 25, 2026
5 checks passed
@ykyohei ykyohei deleted the stimulator_analysis_pipeline branch June 25, 2026 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants