-
Notifications
You must be signed in to change notification settings - Fork 1
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
Implement O3 autofocus on light-sheet arm #48
Conversation
pyproject.toml
Outdated
@@ -31,6 +31,7 @@ dependencies = [ | |||
"nidaqmx", | |||
"iohub==0.1.0.dev3", | |||
"matplotlib", | |||
"pythonnet" |
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.
actually this should be part of copylot.
@edyoshikun it will be great to get your review too, I can't formally request it because you opened this PR |
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.
I am approving based on the stability tests @ieivanov has reported. @edyoshikun can't formally be the reviewer, but Ed should run the code himself to check for reproducibility.
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.
LGTM! I only have a pair of minor non-blocking comments.
examples/test_kim101_stage.py
Outdated
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.
Did I hear correctly that this file wasn't/isn't working? Maybe add a comment, and consider renaming to test_kim101_copylot.py
to match test_kim101_pylablib.py
.
# Refocus O3 | ||
# Some focus_indices may be None, e.g. if there is no sample | ||
valid_focus_indices = [idx for idx in focus_indices if idx is not None] | ||
if valid_focus_indices: |
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.
Cool, I didn't know that empty sequences return false. If this is common knowledge, great! If not, maybe comment?
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.
I think it's relatively common knowledge: https://stackoverflow.com/questions/53513/how-do-i-check-if-a-list-is-empty
examples/test_kim101_stage.py
Outdated
# %% | ||
from copylot.hardware.stages.thorlabs.KIM001 import KCube_PiezoInertia | ||
import time | ||
from copylot import logger |
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.
unused imports logger and time
idx = focus_from_transverse_band( | ||
stack, | ||
NA_det=NA_DETECTION, | ||
lambda_ill=wavelength, |
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.
should we make this also a global for now?
NA_det=NA_DETECTION, | ||
lambda_ill=wavelength, | ||
pixel_size=LS_PIXEL_SIZE, | ||
threshold_FWHM=threshold_FWHM, |
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.
this one as global or at the top of the file?
if np.any(target_z_position > max_z_position) or np.any( | ||
target_z_position < min_z_position | ||
): | ||
logger.error('O3 relative travel limits will be exceeded. Aborting O3 refocus.') |
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.
should this raise Exception or with the logger is fine?
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.
Here we don't want to raise an Exception as that will terminate the acquisition. Our current model is to continue imaging even if O3 autofocus fails. We can consider changing that once we have more trust in the autofocus algorithm such that it quits the acquisition once O3 has reached it's travel limit and cannot be refocused anymore.
mantis/acquisition/acq_engine.py
Outdated
# Define O3 z range | ||
# 1 step is approx 20 nm, 15 steps are 300 nm which is sub-Nyquist sampling | ||
# The stack starts away from O2 and moves closer | ||
z_start = -105 |
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.
perhaps we should rename these z_variables to o3_z_start
. I assume this is a range you found to be good for scanning, but should these variables be globals or parsed from another file?
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.
Renamed as suggested.
Let's see if these values need to change at all. If they are effectively constants, then we can leave them as local variables in the refocus_ls_path
function - they are not used by other functions and I think they'll be easier to find here.
stage : KinesisPiezoMotor | ||
|
||
""" | ||
logger.debug(f'Setting up Kinesis Piezo Motor stage with serial number {serial_number}') |
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.
I assume that the KIM101 has it's own guardrails to know what th maximum_voltage,velocity, and acceleration are. In our case, should we set a separate restriction of a range of velocity and acceleration values that we have calibrated the stage? I think that we should restrict some of these parameters for our usecase.
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.
We don't really expose any way for the users to change these parameters, so I think we can leave this for now
* adding pythonnet and drift correction * removing the pythonnet dependency that should live in copylot * cleanup the demo code * move drift_correction.py to examples * add copylot to dependencies * add O3 stage setup * move serial numbers to acq_engine * rename drift_correction script * add defocus stack acquisition * style * update examples * refactor z stage move * add galvo scanning to acquire_ls_defocus_stack * Update acquire_defocus_stack.py * update examples * switch to pylablib stage and relative moves * add some logging * add KIM101 compensation * fix galvo reset position bug * add find_focus.py example * style * ignore tests in examples dir * move o3 refocus to acq_engine * add checks and logging * debug acq engine o3 refocus * update example * Create separate logs directory * move conda env logger to logs * rename to mantis_acquisition_log * save acquired stacks * implement timed o3 refocus * update kim101 calibration * add relative O3 travel limits * logger fixes * style * add waveorder to deps and format pyproject * update kim101 compensation factor * add threshold and plotting to focus finding * update waveorder dependency * add microscope_operations documentation * update data structure specs * make acquire_ls_defocus_stack MantisAcquisition static method * update examples * rename_kim101 example * rename z_range vars to avoid confusion --------- Co-authored-by: Uditha Velidandla <ivan.ivanov@czbiohub.org>
* adding first prototype of manual registration with napari * adding layer control to hide and show the aligned dataset. * replacing fluor/phase to respective dataset names and saving metadta * fix typo on * fixing channel mixup and adding 3D registration. * z-sampling typo. channels were flipped * fixed focus channel parameters and making this a function to save only the parameters * fixing layer typo * reverting accidental commit with some test * tet for registration.py * adding registration function * adding shape metadata * adding the CLI for applying registration with multiprocessing * adding registration parameter output a more meaningful name * replacing affine transform to similarity transform. this avoids shearing. * adding voxel_size * handle coordinate transformation * formatting isort and black * removing own registration algorithm as scikit image one works * fixes to use only click.echo rather than print * handling float inputs * renaming file to register.py to register and deleting the redundant registration.py * adding refactored register and utils function * renaming chan_1 and chan_2 to phase and fluor * fixing typos to add registration to cli * adding parameters for register.cli that live in utils.py * Fix_deskew_utils (#67) * replacing functinos that live in utils.py * adding utils.py to slurmkit file * adding validation and sorting to parsing.py * update parsing.py for deskew * adding the metadata per position * adding the yaml file writing and parsing * fixing deskew function new utils.py * slurm deskew fixed with new util.py * adding slumkit for registration * Implement O3 autofocus on light-sheet arm (#48) * adding pythonnet and drift correction * removing the pythonnet dependency that should live in copylot * cleanup the demo code * move drift_correction.py to examples * add copylot to dependencies * add O3 stage setup * move serial numbers to acq_engine * rename drift_correction script * add defocus stack acquisition * style * update examples * refactor z stage move * add galvo scanning to acquire_ls_defocus_stack * Update acquire_defocus_stack.py * update examples * switch to pylablib stage and relative moves * add some logging * add KIM101 compensation * fix galvo reset position bug * add find_focus.py example * style * ignore tests in examples dir * move o3 refocus to acq_engine * add checks and logging * debug acq engine o3 refocus * update example * Create separate logs directory * move conda env logger to logs * rename to mantis_acquisition_log * save acquired stacks * implement timed o3 refocus * update kim101 calibration * add relative O3 travel limits * logger fixes * style * add waveorder to deps and format pyproject * update kim101 compensation factor * add threshold and plotting to focus finding * update waveorder dependency * add microscope_operations documentation * update data structure specs * make acquire_ls_defocus_stack MantisAcquisition static method * update examples * rename_kim101 example * rename z_range vars to avoid confusion --------- Co-authored-by: Uditha Velidandla <ivan.ivanov@czbiohub.org> * Update recommended sampling in the LS and LF remote refocus paths (#69) * Update recommended sampling in the LS and LF RR paths Co-Authored-By: Talon Chandler <talonchandler@gmail.com> * Drop mirror scan step by half Co-authored-by: Talon Chandler <talonchandler@gmail.com> --------- Co-authored-by: Talon Chandler <talonchandler@gmail.com> * Improve autofocus reengagement (#74) wait for autofocus to engage * All CLI calls read from zarr (#71) * read from zarr * Use better colors via `iohub` * isort * `estimate_deskew` read from zarr * first-pass standardize register * remove defunct test * fix readme. * standardize estimate_registration * typo * show `mantis -h` in order * standardize register cli * fix parameter * fix estimate-bleaching bug * fix deskew tests * flake8, isort * add registration tests * Update mantis/analysis/AnalysisSettings.py * reducing parameters in config file * re-adding finding best focus slice and rotate image by 90deg * removing test main calls * fixing apply_affine to perform a rotation first before affine. * renaming cli calls and updating README * renaming files for readability and to match CLI * style fix * adding tests and renaming variables for clarity * missed changin the conftest.py * fixing typo on test for test appy cli * modifying code to pass tests * deleting unsed imports flake8 * fix incorrect click.echo * adding pre-affine 90 deg rotationsa bout z as option for cli and croping ROI for focus finding * swapping config to use the indentity matrix as the affine matrix * return the affine transfrom mapping phase to fluorescence * fixing pre_affine_90degree_rotations_about_z in config * renaming and removing redundant printf() * update slurmkit deskew * adding function to read the voxel size from metadata and updating registration slurmkit * voxel_size retreival and modifing apply_affine to be applied to the labelfree channel. * fix typo * updating slurmkit apply_affine * cleanup slurmkit * pass voxel sizes through apply_affine * scaling passes through apply_affine tested * isort * update readme --------- Co-authored-by: Uditha Velidandla <ivan.ivanov@czbiohub.org> Co-authored-by: Talon Chandler <talonchandler@gmail.com>
This PR introduces O3 refocus mechanism to compensate for drift in the O3 position which leads to defocus of images from the LS RR path.
O3 autofocus is achieved by moving the O3
KIM101
stage to axially displace the objective and then using thewaveorder.focus.focus_from_transverse_band
algorithm to find the in-focus slice.Several criteria are implemented to ensure that O3 does not move too close to O2:
plot_path
andthreshold_FWHM
tofocus_from_transverse_band
mehta-lab/waveorder#132, to reject blank stacksThe travel limits we have currently implemented are safe, but when coupled with hysteresis of the O3 stage, limit the total duration of time lapse experiments. In a future PR we can extend the duration of time lapse experiments by:
Resolves #19