-
Notifications
You must be signed in to change notification settings - Fork 6
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
camera exposure time in lightsheet mode #808
Conversation
Co-Authored-By: Annie Wang <6161065+annie-xd-wang@users.noreply.github.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #808 +/- ##
===========================================
+ Coverage 54.26% 54.33% +0.07%
===========================================
Files 154 154
Lines 17489 17444 -45
===========================================
- Hits 9491 9479 -12
+ Misses 7998 7965 -33
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Awesome! That makes sense. Specs for 1H can be found for the Flash (our current assumed default) at https://www.hamamatsu.com/content/dam/hamamatsu-photonics/sites/static/sys/en/manual/C13440-20CU_IM_En.pdf, the Fusion BT at https://www.hamamatsu.com/content/dam/hamamatsu-photonics/sites/static/sys/en/manual/C15440-20UP,-20UP01_IM_En.pdf and the Lightning at https://www.hamamatsu.com/content/dam/hamamatsu-photonics/sites/static/sys/en/manual/C14120-20P_IM_En.pdf. It's worth noting that 1H changes depending on the camera's mode of operation and a few other settings. My original thought was we could just let it throw the error, since I believe this is from our side of the code (which has a fixed rounding tolerate =/= 1H, line 1043 of |
Looking at the code more closely, I don't fully understand why this fixes the error. Is |
@annie-xd-wang, @Sriya-235 and I were going over some of the basics of code review, and I probably went a bit too crazy. So, each type of Hamamatsu Camera has its own 1H/minimum exposure time value. Thus, it seems as if we should have this specified in their very own __init__ functions. Then we were a bit confused as well. Why is it that the methods for max_image_width, which are getters and setters, are both in the parent and child classes? It seems like we could eliminate a fair amount of code if these were only defined in the parent class. We created a new HamamatsuBase class in hopes of simplifying the children classes... For example, I think the calculate_exposure_time is only unique for the Hamamatsu Lightning camera...
@zacsimile, I agree with you that properly_value_step, defined in line 1007 of the HamamatsuAPI, is intended to round up 1H. However, it didn't behave that way the last time I worked with the camera. I will double-check the value of properly_value_step for exposure time the next time I connect to a camera. |
It didn't behave that way for me earlier, but I'm unclear as to why. I wondered if maybe it was returning 1H for a different exposure time. Maybe we need to set exposure time before line interval or vice versa to get the 1H to match. Or the logic is a bit different than I expect in the DCAM API. |
Co-Authored-By: Annie Wang <6161065+annie-xd-wang@users.noreply.github.com>
The minimum exposure time we get from the camera is 1H. Casting the exposure time according to the step size causes accuracy errors. |
Evaluating new light-sheet calculation for ORCA FIRE
Nice find! Super weird that rounding to integer multiples the step size doesn't work, but glad it's working with the minimum now. |
This reverts commit 387e583.
I quickly tried to test it on a Mac, and I got the following error when running it in the continuous mode. However, when I tried to get the same error again, it did not happen. Seems somewhat random. Was in continuous mode, 3 channels selected, hit stop, and it threw the error.
|
|
||
elif sensor_mode == "Light-Sheet": | ||
# Progressive sensor mode operation | ||
readout_time = exposure_time - 1 / (exposure_time + (vn + 10) * h) | ||
readout_time = 0 |
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.
Is this always the case? What happens if the exposure time is very short (e.g., 1 ms)?
devices["camera"][build_ref_name("_", device["type"], int(camera_serial_number, 8))] = camera | ||
try: | ||
oct_num = int(camera_serial_number, 8) | ||
devices["camera"][build_ref_name("_", device["type"], oct_num)] = camera |
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 still need to test this.
#630: The error has been fixed for the Hamamatsu Orca Fire camera. It was due to inaccuracies in exposure time calculation for that specific camera. If the same error occurs on other machines, I'll need to read the specific documentation.