Conversation
talonchandler
marked this pull request as ready for review
August 3, 2026 22:24
Signed-off-by: Sricharan Reddy Varra <sricharan.varra@biohub.org>
Eq. 28 returns a spectral amplitude at the cutoff, which is beta's scale. alpha is added to |OTF|**2, so substituting the same value leaves it ~200x too large. On a confocal OTF with |H(cutoff)| = 0.0106 the Wiener spectral product at the cutoff was 0.0053 instead of 0.171, i.e. ~187 iterations to converge the finest frequencies rather than ~6 -- defeating the point of an unmatched back projector.
calculate_back_projector depends only on the OTF and the rl_bp_* knobs, all fixed for a run, but sat inside apply_inverse_transfer_function and so rebuilt on every call. Callers reconstructing many tiles can now build it once. Output is bit-identical; matched is unaffected.
Signed-off-by: Sricharan Reddy Varra <sricharan.varra@biohub.org>
srivarra
previously approved these changes
Aug 7, 2026
srivarra
left a comment
srivarra
left a comment
Contributor
There was a problem hiding this comment.
I think this looks good to me. Do we want to wait to merge, or just go ahead?
talonchandler
commented
Aug 7, 2026
talonchandler
left a comment
talonchandler
left a comment
Collaborator
Author
There was a problem hiding this comment.
Thanks, left one comment to clean up the parametrization a bit. Otherwise I think we're close to ready.
The RL parameters sat flat alongside the Tikhonov and TV ones, so every generated config carried ten rl_* lines whether or not the algorithm read them, and the list would grow with each new back projector. They now live in an optional RLSettings block that follows the algorithm: filled with defaults for RL/RLGC, dropped with a warning otherwise. A Tikhonov config carries one `rl: null` line instead of ten. Dropping rather than rejecting a stray block keeps the napari plugin working, since it builds widgets from every field and so submits one whatever the algorithm. Two knobs leave the user-facing schema, both still reachable at the model level and both unchanged in behaviour: - rl_bp_order stays at 8. Measured across 2-16 on anisotropic confocal data, 8 sits where the passband has saturated and ringing has not. - rl_bp_resolution_mode stays at 'fwhm'. It declares an instrument class rather than tuning anything; 'fwhm_over_sqrt2' is for resolution-doubling optics such as iSIM. to_model_kwargs() is the seam between the nested config and the flat model signatures. Every apply_inverse call site uses it, including phase and birefringence, so a nested block added there later is picked up without touching the callers. Also corrects the shared reconstruction_algorithm description, which still advertised only Tikhonov and TV after RL/RLGC were added. That string regenerates into the phase and birefringence example configs. Refs #573
The previous commit widened the shared reconstruction_algorithm description to mention RL/RLGC. That field is on FourierApplyInverseSettings, so the text regenerated into the phase and birefringence example configs -- modalities that raise NotImplementedError for RL/RLGC, where the original 'Tikhonov' or 'TV' wording was already correct. Fluorescence overrides the field with its own description, so its configs were never affected either way.
Contributor
|
Here's an example of an updated yaml config for RL: input_channel_names: [GFP]
time_indices: all
reconstruction_dimension: 3
fluorescence:
transfer_function:
yx_pixel_size: 0.1 # scalar, or {y: 1.018, x: 0.1842} for anisotropic
z_pixel_size: 0.25
z_padding: 0
index_of_refraction_media: 1.3
numerical_aperture_detection: 1.2
wavelength_emission: 0.532
confocal_pinhole_diameter: null # null = widefield
apply_inverse:
reconstruction_algorithm: RL
rl:
iterations: 100
back_projector: matched
background: 0.0
stopping_tolerance: nulland if you want to use Tikhonov: input_channel_names: [GFP]
time_indices: all
reconstruction_dimension: 3
fluorescence:
transfer_function:
yx_pixel_size: 0.1
z_pixel_size: 0.25
z_padding: 0
index_of_refraction_media: 1.3
numerical_aperture_detection: 1.2
wavelength_emission: 0.532
confocal_pinhole_diameter: null
apply_inverse:
reconstruction_algorithm: Tikhonov
regularization_strength: 0.001So if a user runs RL they have another block of parameters to set, but if they don't, it doesn't need to be filled in. |
Widening the shared FourierApplyInverseSettings to accept "RL"/"RLGC" let a phase or birefringence config validate and then fail deep in the reconstruction, after the transfer function had been computed. Keep the shared model to the Fourier filters and let fluorescence widen it in its own subclass. Two more pairings are now caught while parsing rather than mid-run: 2D fluorescence with RL/RLGC (checked on ReconstructionSettings, the only model that sees reconstruction_dimension), and RLGC with an unmatched back projector. The model-level NotImplementedError guards stay for direct callers.
…ters reconstruct() computes the transfer function itself and has no back_projector_otf parameter, so documenting one promised a knob that does not exist. Point callers who want to reuse a back projector at the calculate/apply pair instead, which is where the parameter lives.
Collaborator
Author
|
LGTM! Thanks @srivarra...made a couple minor changes, and I think it's ready for final review and merge. |
srivarra
approved these changes
Aug 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds Richardson-Lucy (RL) and Gradient-Consensus (RLGC) deconvolution for 3D fluorescence.
Select it in a config with
reconstruction_algorithm: RLorRLGC, and tunerl_iterations,rl_background, andrl_stopping_tolerance. RLGC resists the noise overfitting ("starry night") that RL shows when over-iterated.RL/RLGC options raise
NotImplementedErrorfor 2D fluorescence and for phase/birefringence.The operator-agnostic solver lives in
waveorder/rlgc.py, adapted from Andrew York's Gradient Consensus demo (doi.org/10.5281/zenodo.10278918). Tests intests/models/test_rlgc.pycover the adjoint operators, bead sharpening under Poisson noise, RL vs RLGC over-iteration, and theNotImplementedguards.