Skip to content
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

Fix silent calibration failures #214

Merged
merged 26 commits into from
Oct 3, 2022
Merged

Fix silent calibration failures #214

merged 26 commits into from
Oct 3, 2022

Conversation

talonchandler
Copy link
Collaborator

The following figure summarizes the fix in this PR:

pr-plot

Caption: before this PR the calibrations were being optimized over a region of fixed width = 0.05 waves. For small swing values (swing = 0.04 pictured here), the optimization region crossed the extinction state which often resulted in calibrated states on the wrong side of the extinction state.

This PR scales the optimization with the swing so that the calibrated states are always on the correct side of the extinction state.

Details: I've changed the LCA search region from a fixed 0.05 to min(swing/ratio, 0.05) for 4-state and min(swing, 0.05) for 5-state calibrations. This means that the behavior for large swings (>0.05 for 5-state and >0.1 for 4-state) will be unchanged---only small-swing calibrations are affected by this PR.

More data: this PR also includes an automation script that I used to collect 40 calibrations in all combinations of:

  • [4, 5] state
  • [0.1, 0.03, 0.01, 0.005] swing
  • 5 repeats

Below are the LCA/LCB plots which show:

  • that none of the calibrations failed---all of the states are placed on the correct side of the extinction state
  • that the LC states vary by ~0.01 waves in LCA/LCB space. More work is needed to characterize the major contributors to this uncertainty.

Caption: Columns correspond to decreasing swing left to right. First 5 rows correspond to repeats of the 4-state algorithm and the last 5 rows correspond to repeats of the 5-state algorithm.
4-state
5-state

@codecov-commenter
Copy link

codecov-commenter commented Oct 1, 2022

Codecov Report

Merging #214 (4f14502) into main (b99c40b) will decrease coverage by 0.85%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #214      +/-   ##
==========================================
- Coverage   11.02%   10.17%   -0.86%     
==========================================
  Files          45       39       -6     
  Lines        6087     5190     -897     
==========================================
- Hits          671      528     -143     
+ Misses       5416     4662     -754     
Impacted Files Coverage Δ
recOrder/plugin/workers/calibration_workers.py 0.00% <0.00%> (ø)
recOrder/scripts/calibrate.py 0.00% <0.00%> (ø)
recOrder/tests/conftest.py 97.53% <0.00%> (-0.25%) ⬇️
recOrder/io/utils.py 0.00% <0.00%> (ø)
recOrder/scripts/cli.py 0.00% <0.00%> (ø)
recOrder/io/config_reader.py 0.00% <0.00%> (ø)
recOrder/calib/Calibration.py 0.00% <0.00%> (ø)
recOrder/io/core_functions.py 0.00% <0.00%> (ø)
recOrder/io/zarr_converter.py 0.00% <0.00%> (ø)
recOrder/pipelines/qlipp_pipeline.py 0.00% <0.00%> (ø)
... and 18 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@deprecated-napari-hub-preview-bot
Copy link

deprecated-napari-hub-preview-bot bot commented Oct 1, 2022

Preview page for your plugin is ready here:
https://preview.napari-hub.org/mehta-lab/recOrder/214
Updated: 2022-10-03T15:33:27.504734

@ziw-liu ziw-liu added the bug Something isn't working label Oct 1, 2022
Copy link
Contributor

@ziw-liu ziw-liu left a comment

Choose a reason for hiding this comment

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

@talonchandler How do you feel about a more informative filename such as debug_calibration.py for the new script? Other than that I think we are good to merge.

@talonchandler
Copy link
Collaborator Author

Good call...fixed.

@talonchandler talonchandler merged commit f7946b9 into main Oct 3, 2022
@talonchandler talonchandler deleted the small-swing-fix branch October 3, 2022 15:44
@talonchandler talonchandler restored the small-swing-fix branch October 3, 2022 16:25
@ieivanov
Copy link
Contributor

ieivanov commented Oct 6, 2022

Thanks for putting in this work @talonchandler! I liked your repeat_calibration.py script. Seems like the qbutton_gui_mode button need a better name :)

As you say, from the repeated calibrations you did, I think it's clear that our calibration algorithm needs a bit more work. (2,4), (6,4), and (9,3) are hardly examples of good calibration. At this point, I don't have a clear sense of where these discrepancies may come from. Do you know if the optimization algorithm reached approximately equal intensities in all cases? One thing I've been suspicious about is whether we allow enough time for the LCs to settle into a new state. For example, in opt_I0 we snap an image right after changing the LC state.

self.define_lc_state('State1', self.lca_0, self.lcb_0)
intensity = snap_and_average(self.snap_manager)

define_lc_state calls waitForDevice or waitForConfig, but I'm not sure if these really give the LCs enough time to settle. I think the LCs are well equilibrated by that point - the last steps of the calibration algorithm don't change the intensity much, but we could try giving it a bit more time.

@talonchandler talonchandler deleted the small-swing-fix branch October 6, 2022 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Calibration algorithm fails silently with small swing (swing <0.05 for 4-state and <0.01 for 5-state)
5 participants