-
Notifications
You must be signed in to change notification settings - Fork 3
fix: CLSM get_fluorescence_decay and get_phasor #49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
Conversation
…ecision loss in get_phasor - get_fluorescence_decay: when stack_frames=true, now correctly processes all frames instead of only the first frame. Uses target_frame index to write all photons to frame 0 of output while iterating over all n_frames. - get_phasor: fix precision loss in non-stacked branch where phasor values (floats between -1 and 1) were cast to int, truncating all precision. Changed static_cast<int> to static_cast<float>. - Update test reference files to reflect correct stacked decay behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 366b1e2aa6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…y for irregular frames
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request fixes two bugs in CLSM (Confocal Laser Scanning Microscopy) analysis methods related to frame stacking and numerical precision.
Changes:
- Fixed
get_fluorescence_decayto correctly accumulate data from all frames whenstack_frames=True, instead of only processing the first frame - Improved numerical precision in
get_phasorby casting tofloatinstead ofint, preventing truncation of decimal values - Updated test reference data to reflect the corrected output
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thanks for the PR and the detailed description and validation against the BH vendor exports. I am currently planning a new release and will integrate your changes as part of that effort. The upcoming release focuses mainly on improved memory management (no API changes) and is a slightly larger update; current ETA is end of February. I will keep the pull request open for now, integrate your changes internally, and then merge your PR as part of the next release. Afterwards, I will take care of publishing the update to conda-forge and pip (on my personal TODO list). Thanks again for the careful implementation! |
Summary
get_fluorescence_decayto correctly handlestack_frames=Trueby accumulating data from all frames into the first output frame.get_phasorby usingfloatinstead ofintfor result storage.