roflmaostc / DeconvOptim.jl

A multi-dimensional, high performance deconvolution framework written in Julia Lang for CPUs and GPUs.
https://roflmaostc.github.io/DeconvOptim.jl/stable/
MIT License
61 stars 6 forks source link

Misleading `generate_psf` function #44

Closed kunzaatko closed 1 year ago

kunzaatko commented 1 year ago

generate_psf gives a fftshifted PSF instead of what is expected. If this will be hard to fix if intended so, since I think that it is passed this way throughout the whole package, meaning that everywhere, where the PSF is ffted, the output is expected to be non-fftshifted.

An alternative would be to point this out in the documentation, but this does make it non-intuitive for using PSFs generated through a model or another package since they would have to be fftshifted, before passed to the

roflmaostc commented 1 year ago

Can you specify what exactly is misleading?

I agree, that generate_psf could be avoided to export but it also does not hurt much. For more advanced PSFs see https://github.com/RainerHeintzmann/PointSpreadFunctions.jl

roflmaostc commented 1 year ago

Ah you edited it :smile:

Yes, DeconvOptim.jl expects all PSFs to be centered around (1,1,1) (btw, I would call this ifftshifted). Back then I liked that convention more than having the center in the middle.

I'm a little afraid to change it since it would imply big changes everywhere

kunzaatko commented 1 year ago

Yeah... I was afraid of that.

roflmaostc commented 1 year ago

Do you want to prepare a PR to maybe add some clarification here?

https://github.com/roflmaostc/DeconvOptim.jl/blob/4c30146df86c28ebcc0cd017eeba7bea9b71d27f/src/deconvolution.jl#L6

and here?

https://github.com/roflmaostc/DeconvOptim.jl/blob/4c30146df86c28ebcc0cd017eeba7bea9b71d27f/src/utils.jl#L254

kunzaatko commented 1 year ago

For more advanced PSFs see https://github.com/RainerHeintzmann/PointSpreadFunctions.jl

I didn't know about this! I am in the process of creating my own package... Will look into this one, though. I created a repo for it now, so you can take a look if you want to: https://github.com/kunzaatko/TransferFunctions.jl

roflmaostc commented 1 year ago

Ah! Tagging @RainerHeintzmann that he's aware of your package too

kunzaatko commented 1 year ago

Would this be alright as a note to the user?

kunzaatko commented 1 year ago

Oh, my... Now I see that the code was formatted. Sorry for that... Should I undo that?

roflmaostc commented 1 year ago

Maybe add the tabs to the function arguments back. Otherwise they are fine. Tests currently fail because of a test dependency. Fix it later.

Thanks!

RainerHeintzmann commented 1 year ago

Happy, if you want to discuss about PSF generation rouitines. There is quite some work that went into that package. You may also be interested in https://github.com/bionanoimaging/PSFDistiller.jl. The ifftshift is a problem, but it would not be too hard to provide some adaptor methods, which are more user friendly in this aspect.

kunzaatko commented 1 year ago

@RainerHeintzmann I would very much like to talk about PSF generation routines! I am in a rush right now and since I know how it works, I will stick to my package for now. But I will take some inspiration (maybe some code... would you be OK with that?), since I wanted to try out Zernike polynomials anyway. I guess I will be having some spare time in the middle of August, then maybe we can talk about that and possibly merge the two packages together (maybe under JuliaMicroscopy?)... What do you think about that?

RainerHeintzmann commented 1 year ago

Sure. Lets talk in August.