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

Adaptive Optics final merge (at the buzzer) #699

Merged
merged 57 commits into from
Dec 4, 2023

Conversation

conorhughmcfadden
Copy link
Collaborator

Merging all of the Adaptive Optics code. If there is a mirror defined in the config file, an "Adaptive Optics" option appears in the feature menu, which opens a GUI capable of doing iterative AO.

Main AO routine in adaptive_optics.py is very similar to the routine found in autofocus.py. Main API is in imagineoptics/imop.py, custom built from ctypes using the manufacturer DLL.

Mirror is a Mirao52e from ImagineOptics, using the WaveKit API.

conorhughmcfadden and others added 30 commits January 17, 2023 15:03
Initial Commit. Only Commit :)
#309
- Needed to modify camera_base, HamamatsuCamera to include camera_id -> There is new functionality in CameraBase which needs to know the serial number

- Modified adaptiveoptics.py feature: "run_single_channel_acquisition_with_features" doesn't exist anymore. signal_thread just calls run_acquisition now...
Some minor typos...

AO Feature works now with new merge!
ttk.Combobox which allows AO correction from zero ("flat") or current modes ("current")
Solved conflicts not caught by Git
Got rid of a bunch of extra print statements I'd added, etc
I had the cameras loading separately outside of device_ref_dict into a list. Needed to manually remove this as Git didn't catch it.
Again, stuff Git didn't catch
SyntheticMirror needed for model/launch_virtual_microscope
Got rid of id. Not needed anymore.
Added AdaptiveOpticsPopupController to sub_controllers __init__.py
Imports and menu adds now handled in menu_controller.py
Fixed circular import error in subcontroller __init__: MenuController MUST be last
AO signal and data threads weren't working as intended... Caused major frame/mirror update sync issues.

Fix was basically:

- mirror.flat() before mirror.display_modes in func_signal() >> otherwise mirror updates were somtimes unsuccessful

- Allow in_func_signal handling mirror updates to run freely instead of waiting for data_queue, a la autofocus.py

- Keep track of frames_done in func_data() and use it to define the stop condition >> Some frames are dropped if signal and data threads get out of sync

- Remove set number of frames in model.data_thread >> frames_done is typically less than frame_num, causing early stopping/loss of data_thread before signal_thread is finished

- Manually stop the acquisition in end_func_data()

AO routine now appears to run super fast with correct mirror updates
If mirror is synthetic, don't even add the Adaptive Optics menu option...
In microscope >> assemble_device_config_lists...

- try to grab the device from self.config
- if KeyError: 'mirror', manually add synthetic mirror to self.config and call the function again recursively
Copy link

codecov bot commented Dec 1, 2023

Codecov Report

Attention: 562 lines in your changes are missing coverage. Please review.

Comparison is base (4acd050) 61.66% compared to head (0a116a9) 60.14%.
Report is 41 commits behind head on develop.

Files Patch % Lines
src/aslm/model/features/adaptive_optics.py 10.00% 225 Missing ⚠️
...sub_controllers/adaptiveoptics_popup_controller.py 16.04% 136 Missing ⚠️
src/aslm/view/popups/adaptiveoptics_popup.py 14.96% 108 Missing ⚠️
src/aslm/model/device_startup_functions.py 11.53% 23 Missing ⚠️
src/aslm/controller/controller.py 43.58% 22 Missing ⚠️
src/aslm/model/model.py 30.76% 18 Missing ⚠️
...troller/sub_controllers/waveform_tab_controller.py 0.00% 8 Missing ⚠️
...aslm/controller/sub_controllers/menu_controller.py 41.66% 7 Missing ⚠️
src/aslm/model/devices/mirrors/mirror_base.py 45.45% 6 Missing ⚠️
src/aslm/model/devices/galvo/galvo_base.py 20.00% 4 Missing ⚠️
... and 2 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #699      +/-   ##
===========================================
- Coverage    61.66%   60.14%   -1.52%     
===========================================
  Files          145      151       +6     
  Lines        14854    15631     +777     
===========================================
+ Hits          9160     9402     +242     
- Misses        5694     6229     +535     
Flag Coverage Δ
unittests 60.14% <19.25%> (-1.52%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AdvancedImagingUTSW
Copy link
Collaborator

The only major thing that pops out to me is that the logic for the mirror is in the controllers, not the model. This isn't how we have traditionally done it. Typically all of the logic and hardware interactions are done in the model. The controller just communicates between the view and the model. Also, some of the basic commands in the adaptive optics feature could be placed in the tools section.

I didn't completely finish documenting the feature, had to jump on the flight.

@zacsimile
Copy link
Collaborator

Confirmed working on the mesoSPIM BT.

Copy link
Collaborator

@annie-xd-wang annie-xd-wang left a comment

Choose a reason for hiding this comment

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

The code looks fine to me. I changed two things: 1) It won't load a synthetic mirror if there is no mirror in the configuration yaml file. 2) The code to change the camera. The two changes will not break adaptive optics, but running it on a machine to double-check is better.

@AdvancedImagingUTSW AdvancedImagingUTSW merged commit 96762e6 into develop Dec 4, 2023
1 check passed
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.

4 participants