czbiohub-sf / shrimPy

shrimPy: Smart High-throughput Robust Imaging & Measurement in Python
BSD 3-Clause "New" or "Revised" License
8 stars 1 forks source link

PSF measurement, simulation, and deconvolution #130

Closed talonchandler closed 3 months ago

talonchandler commented 9 months ago

TODO:

ieivanov commented 3 months ago

@talonchandler would you like to keep scripts/fit_psf_to_beads.py, scripts/simple_psf.py, and scripts/simulate_psf.py? They may need to have their deskew updated. These scripts call on deskew/_deskew_matrix which I think now is deprecated. Or is deskew/_get_transform_matrix the deprecated one? deskew_data uses

    matrix = _get_transform_matrix(
        raw_data.shape, ls_angle_deg, px_to_scan_ratio, keep_overhang
    )

but I think that may be a bug? I'd like to remove one of _deskew_matrix or _get_transform_matrix.

ieivanov commented 3 months ago

P. S. If that's a bug in the deskew, let's fix it in a separate PR that we can merge right away

ieivanov commented 3 months ago

P. S. If that's a bug in the deskew, let's fix it in a separate PR that we can merge right away

This is a bug in this branch only, I'll fix it

talonchandler commented 3 months ago

I removed scripts/fit_psf_to_beads.py, scripts/simple_psf.py, and scripts/simulate_psf.py because I don't think we'll need them. I can resurrect them if that changes.

Those are the only places that _deskew_matrix is used, so I've deleted that function, too.

With those deletions we can focus our attention on the _get_transform_matrix, which was regressing from the main branch (the GPU deskew branch changed the affine transformation matrix). I added a test to catch this problem, then fixed the problem.

I spot-tested a deskew on real data, so I now think the deskew in this branch is ready (a minor refactor from main)

@ieivanov thanks for flagging this. I just took a look, and I think these deskew functions were the only places where accidental regressions happened.

ieivanov commented 3 months ago

I'll test the refactored measure_psf today and then we can merge

talonchandler commented 3 months ago

LGTM!