mehta-lab / recOrder

3D quantitative label-free imaging with phase and polarization
BSD 3-Clause "New" or "Revised" License
11 stars 4 forks source link

[FEATURE] Refactor recOrder calibration #120

Open ieivanov opened 2 years ago

ieivanov commented 2 years ago

Refactor recOrder calibration module such that

cc: @AhmetCanSolak

AhmetCanSolak commented 2 years ago

hello @ieivanov ,

Thank you for starting this discussion here. I quickly went over the the recOrder.calib module and I believe it can become independent from acquisition software or voltage control devices. I felt like in the existing code many places can become more independent. An example: https://github.com/mehta-lab/recOrder/blob/b5328d2ed4b8c2deb596988a11954e76c163adae/recOrder/calib/Optimization.py#L13-L25

This small method of the BrentOptimizer can take current_lca and current_lcb values as arguments then this method wouldn't need to know about self.calib.mmc. We can apply similar thinking on other parts too and achieve what you desire.

Regarding your third point:

it can be imported and used by external libraries

I like to propose hosting new calibration module in coPylot at root of the package(something like copylot.calib) instead of having new calibration code as a dependency and importing it(in a way better to have less packages to maintain IMO). We can still make the calibration code lighter so it can be imported on other places from copylot when needed easily yet code can live in copylot repository so we know where to find and follow changes on it easily. Looking forward to hearing what do you think about this @ieivanov ?

ieivanov commented 2 years ago

This small method of the BrentOptimizer can take current_lca and current_lcb values as arguments then this method wouldn't need to know about self.calib.mmc. We can apply similar thinking on other parts too and achieve what you desire.

Totally agree. In this example also, the 0.01 and 1.6 limits are hardcoded, it would be helpful if these are defined as constants (LC_RETARDANCE_MIN and LC_RETARDANCE_MAX at the top of the class.

More generally, I think the recOrder.calib module should be responsible for choosing the next LC state in the optimization procedure given the current LC state and measured image intensity. Actually changing the LC state and acquiring images at those new settings should happen outside of recOrder.calib. Does that make sense? Do you agree @AhmetCanSolak @mattersoflight and @talonchandler? This will allow us to make LC optimization, data acquisition, and hardware control independent. We'll have to see how feasible that is.

I like to propose hosting new calibration module in coPylot at root of the package(something like copylot.calib)

@mattersoflight and @talonchandler may have an opinion on this. I see coPylot as an advanced form of microscope control. It would still be helpful for us to calibrate and acquire images using MM and recOrder alone. Brent and MinScalar optimization are pretty generic, but something like https://github.com/mehta-lab/recOrder/blob/b5328d2ed4b8c2deb596988a11954e76c163adae/recOrder/calib/Calibration.py#L362-L411 seems pretty specific to QLIPP imaging. I welcome more discussion here

talonchandler commented 2 years ago

@ieivanov I agree with the main proposal: recOrder's calibration logic can be refactored so that it becomes independent of the LC control device and acquisition software.

As I understand it, you're prepping this code to be reused with coPylot on mantis...is that correct? Do your plans need polarization functionality beyond QLIPP? Do you expect to use a different optimizer and opt_I60 (and related) functions?

Re moving pieces of the calibration code to coPylot: I'm not strongly opposed, but I'm not sure I see the benefit at this point. I don't think there's enough calibration code to warrant its own repo, so I think we either have to choose coPylot imports recOrder or vice versa. I lean towards keeping the calibration code in recOrder unless there's a benefit I'm missing.

Thanks!

ieivanov commented 2 years ago

As I understand it, you're prepping this code to be reused with coPylot on mantis...is that correct? Do your plans need polarization functionality beyond QLIPP? Do you expect to use a different optimizer and opt_I60 (and related) functions?

You're correct. Using coPylot on mantis is motivated by the need for fast acquisition along two independent light paths. In the not far future, we may extend this to the Dragonfly as well, where we are planning to acquire fluorescence (spinning disk confocal) and LF data (phase and polarization). At present, I don't see the need to do anything more than QLIPP, i.e. calibration and acquisition using 4-5 LC states. The current optimization methods works well (note: I may do a slight bit of tuning, for example the extinction calibration acquires multiple images that have the same intensity, meaning that the LC voltage is changed insignificantly).

Re moving pieces of the calibration code to coPylot: I'm not strongly opposed, but I'm not sure I see the benefit at this point. I don't think there's enough calibration code to warrant its own repo, so I think we either have to choose coPylot imports recOrder or vice versa. I lean towards keeping the calibration code in recOrder unless there's a benefit I'm missing.

I think I agree with you, @AhmetCanSolak what do you see as the benefit of moving some of this code to coPylot?

AhmetCanSolak commented 2 years ago

Re moving pieces of the calibration code to coPylot: I'm not strongly opposed, but I'm not sure I see the benefit at this point. I don't think there's enough calibration code to warrant its own repo, so I think we either have to choose coPylot imports recOrder or vice versa. I lean towards keeping the calibration code in recOrder unless there's a benefit I'm missing.

I think I agree with you, @AhmetCanSolak what do you see as the benefit of moving some of this code to coPylot?

To put it simply, it is one less package to maintain. coPylot will be soon on PyPI and already gets its dependencies from PyPI. As @talonchandler mentioned, there is not enough calibration code to make it a package/repository separately as far as I see. We are and will be maintaining coPylot. So these calibration related code pieces can get CI/CD(code style/linting/tests/releases) for free from coPylot. So less maintenance effort on your end. Yet we can enable you to call/import only calibration related code from coPylot when you need, so you can use it with MM and recOrder when you want to. Also from coPylot perspective, it is one less dependency.

mattersoflight commented 2 years ago

It is very exciting to make recOrder independent of acquisition platform. Thanks for thinking through this together @ieivanov , @talonchandler , @AhmetCanSolak .

Re moving pieces of the calibration code to coPylot: I'm not strongly opposed, but I'm not sure I see the benefit at this point. I don't think there's enough calibration code to warrant its own repo, so I think we either have to choose coPylot imports recOrder or vice versa. I lean towards keeping the calibration code in recOrder unless there's a benefit I'm missing.

I think I agree with you, @AhmetCanSolak what do you see as the benefit of moving some of this code to coPylot?

To put it simply, it is one less package to maintain. coPylot will be soon on PyPI and already gets its dependencies from PyPI. As @talonchandler mentioned, there is not enough calibration code to make it a package/repository separately as far as I see. We are and will be maintaining coPylot. So these calibration related code pieces can get CI/CD(code style/linting/tests/releases) for free from coPylot. So less maintenance effort on your end. Yet we can enable you to call/import only calibration related code from coPylot when you need, so you can use it with MM and recOrder when you want to. Also from coPylot perspective, it is one less dependency.

The calibration and background correction algorithms together compensate for aberrations in light path. From the point of view of having clean dependencies, it is optimal to keep all the label-free imaging calibration algorithms in recOrder, and make them agnostic to the platform used to acquire data (MM or coPylot).

It does make sense to implement "device adapter interface" to set LC voltages (either via controller API, via TriggerScope, or via NI DAQ) in the acquisition software. Ahmet Can, there may be a need for thin layer around Meadowlark device adapters in coPylot - equivalent to this MM device adapter. In this case, coPylot will still be a dependency for recOrder when it is the acquisition platform, instead of pycro-manager/micro-manager combination.

PS: Talon maintains the recOrder pyPI package and napari plugin . It is worth discussing on a separate issue how maintenance of recOrder can be improved with CI/CD strategies developed for coPylot.

AhmetCanSolak commented 2 years ago

hello @mattersoflight !

I thought @ieivanov 's initial suggestion was factoring out the recOrder calibration module out into an independent package. Maybe I misunderstood? please correct me @ieivanov if so. That's why I was suggesting calibration code to live in coPylot codebase instead of being an independent package. With calibration code living in recOrder (and as soon as recOrder is maintained on PyPI), we can have recOrder as an optional dependency(as some coPylot users might not be interested in label free imaging goodies) to use together with coPylot.

Also happy to have a separate discussion on CI/CD strategies to learn from each other with @talonchandler .

ieivanov commented 2 years ago

I thought @ieivanov 's initial suggestion was factoring out the recOrder calibration module out into an independent package. Maybe I misunderstood?

Ah, I see - no, that's not what I meant. My suggestion was to separate, within recOrder, the bits of the calibration pipeline that are 1) responsible for LC control, 2) responsible for image acquisition, and 3) responsible for optimizing the LC state based on acquired image data. We can refactor the code to make it more modular and easier to reuse in different project, which I think is something that we all agree on.

AhmetCanSolak commented 2 years ago

Thanks for the clarification @ieivanov !

Where shall we start? any of you started anything yet @ieivanov @talonchandler ? Feel free to give me some leads so I can dive into refactoring and we can start moving forward on this.

mattersoflight commented 11 months ago

@talonchandler , @ieivanov , @edyoshikun , @ziw-liu and I revisited the separation of calibration and reconstruction in recOrder 1.0.0. Here is a summary:

Why separate calibration and reconstruction

recOrder enables correlative fluorescence and label-free imaging with inexpensive widefield microscopes augmented with modular hardware and physics-informed algorithms. For the software will be maximally useful if it is as modular as the hardware. Our current area of application is high-throughput imaging of cells and zebrafish with microscopes that use diverse acquisition stacks.

How to separate calibration and reconstruction

3D fluorescence and phase imaging use hardware that is commonly found on microscopes and can be considered pure analysis algorithms. Polarization imaging requires communication with custom hardware (LC, shutter, camera). The key observation from our brainstorm today is that only calibration algorithms need to connect directly with the hardware. Once the hardware is calibrated, the acquisition engine can, in principle, treat the polarization channels as any other channel.

For hardware communication during calibration, we decided to leverage our investment in Micro-Manager device adapter for LCs, and depend directly on the Micro-Manager core, instead of the Micro-Manager studio.

The reconstruction algorithms can listen to the underlying data and trigger reconstructions when the "unit of analysis" is written to the disk.

Open questions related to the implementation: