-
Notifications
You must be signed in to change notification settings - Fork 5
LF only acquisition with Mantis hardware. #192
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
LF only acquisition with Mantis hardware. #192
Conversation
This reverts commit 1748131.
…ting when copying the events
…ccess Oryx camera using mmc.getCameraDevice() instead of hard-coded string
|
Note: The author is wrong for some of these commits because the default user for the labelfree user on Mantis is |
|
I am unable to reproduce the pytest failure: |
| mm_config_file=mm_config_filepath, | ||
| lf_config_file=lf_config_filepath, | ||
| demo_run=demo_run, | ||
| enable_ls_acq=False, |
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.
Fine for now, eventually we'd like to be able to determine that from the acquisition config (settings) 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.
Ok. We'll put this on the to-do list for later. I have also been thinking that rather than instantiating 2 acquisitions, and disabling one, we could also just not instantiate the 2nd one... and that would mean that there would be now "disabled" state.
|
|
||
| # Determine label-free acq timing | ||
| oryx_framerate = float(self.lf_acq.mmc.getProperty('Oryx', 'Frame Rate')) | ||
| oryx_framerate = float( |
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.
Unfortunately, there is no standard way to get the camera framerate in MicroManager - this will work for Spinnaker cameras which have a Frame Rate property exposed through the device adapter. Other camera manufacturers may call it something different. Still. I do agree with the change you are making here as self.lf_acq.mmc.getCameraDevice() is more general than Oryx. As a step further, maybe we can make a microscope_operations.get_camera_framerate() function which will standardize retrieving the camera framerate across the few different camera manufacturers we use at the Biohub?
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 is a pretty weak suggestion at this point - we have hardcoded device name at many places throughout the code, we can also clean these up in another PR, I know this is not the point of 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.
Yes, let's note that for now, and deal with that when we're using a different camera.
| # ls_image_saved_fn = check_ls_acq_finished | ||
|
|
||
| # Generate LF acquisition events | ||
| # Generate LF MDA |
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.
Do we need to check if these are enabled?
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.
it doesn't crash anything to generate an MDA that you're not going to run... but it is wasted computation and memory.
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 I'd like to move this functionality inside of each acquisition class eventually... so I'll handle it then.
ieivanov
left a 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 this looks good to me. I agree that is resolves #184. Let's aim to get to a point where we can stress test the pymmcore-plus acquisition engine on the mantis microscope with minimal software polishing to confirm that pymmcore-plus can do everything we need it. This will involve running a typical mantis acquisition (two arms, multiple channels, few hours imaging multiple positions) and an acquisition where we only use the LF arm. I think the only step towards that goal is finding a good data writer, correct?
|
Let me know if you need to me merge these changes, I can overwrite the failed tests check |
36930a3
into
czbiohub-sf:170-migrate-to-pymmcore-plus
Addresses: #179 (partial) & #184