-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Add blind Richardson-Lucy deconvolution (supersedes #3524) #4717
base: main
Are you sure you want to change the base?
Add blind Richardson-Lucy deconvolution (supersedes #3524) #4717
Conversation
Co-Authored-By: anki-xyz <11031615+anki-xyz@users.noreply.github.com>
Co-Authored-By: anki-xyz <11031615+anki-xyz@users.noreply.github.com>
Co-Authored-By: anki-xyz <11031615+anki-xyz@users.noreply.github.com>
Co-Authored-By: anki-xyz <11031615+anki-xyz@users.noreply.github.com>
Co-Authored-By: anki-xyz <11031615+anki-xyz@users.noreply.github.com>
…deconvolution # Conflicts: # skimage/restoration/deconvolution.py
Co-Authored-By: anki-xyz <11031615+anki-xyz@users.noreply.github.com>
Hello @bioimage-analysis! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
Comment last updated at 2020-05-26 16:29:03 UTC |
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 made a quick reading to solve the CI issue.
There is still a failure in our CI. See https://travis-ci.org/github/scikit-image/scikit-image/jobs/687078292 I encourage you to build and run the test on your machine. Please, refer to https://scikit-image.org/docs/stable/contribute.html#build-environment-setup |
@sciunto sorry for the delay, things are reopening and I am running a little out of time. To test blind richardson, we need to fetch "reconstruction_blind_RL.npy", but the file is not on the scikit-image server so I am not sure how to deal with that with the function |
No problem! You need to install |
@sciunto sorry, I was not clear, everything work well on my computer when I use a local path, the probably is if I try to fetch the file on scikit-image, it doesn't work. Here is the error I get: KeyError Traceback (most recent call last)
<ipython-input-20-0de14c355e12> in <module>
----> 1 test_blind_richardson_lucy()
<ipython-input-6-cc4e3b6eb27d> in test_blind_richardson_lucy()
30 iterations=iterations)
31
---> 32 path = fetch('restoration/tests/reconstruction_blind_RL.npy')
33 #path = 'skimage/restoration/tests/reconstruction_blind_RL.npy'
34
~/Desktop/scikit-image/skimage/_shared/testing.py in fetch(data_filename)
214 """Attempt to fetch data, but if unavailable, skip the tests."""
215 try:
--> 216 return data._fetch(data_filename)
217 except ConnectionError:
218 pytest.skip(f'Unable to download {data_filename}')
~/Desktop/scikit-image/skimage/data/__init__.py in _fetch(data_filename)
125 """
126 resolved_path = osp.join(data_dir, '..', data_filename)
--> 127 expected_hash = registry[data_filename]
128
129 # Case 1:
KeyError: 'restoration/tests/reconstruction_blind_RL.npy' |
@bioimage-analysis you need to add a registry entry in |
@bioimage-analysis it's looking good! =D |
@jni should we host the file in https://gitlab.com/scikit-image/data ? |
Lucy algorithm as described in Fish et al., 1995. | ||
It is an iterative process where the PSF | ||
and image is deconvolved, respectively. | ||
It is more noise tolerant than other algorithms, |
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 would add blank lines between paragraphs here.
Yes I think so! It looks like the documentation build is failing:
Have you tried making the docs locally? I can't say that I understand the sphinx error because the lines don't correspond to the lines in the source .py file. I guess the best way is to attempt the build and then inspect the generated .rst file. |
Having said this, 156KB is not a huge deal for this repo, and this PR has taken a long time already, so I wouldn't hold it up for this. |
Getting closer, thank you for the advice @jni |
@bioimage-analysis @jni @sciunto thanks for taking care of this PR! |
========================= | ||
Image blind Deconvolution | ||
========================= | ||
We present here a usage example of a blind deconvolution algorithms |
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.
We present here a usage example of a blind deconvolution algorithms | |
We present here a usage example of a blind deconvolution algorithm |
|
||
# Create the PSF | ||
psf_gaussian = np.zeros_like(noisy_image) | ||
w, h = noisy_image.shape |
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.
the names are misleading here, if they mean width and height, since it's the other way around.
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 think you can just invert them :-)
"""Returns a callback function to store the evolution of the level sets in | ||
the given list. | ||
""" | ||
|
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.
Thanks a lot @bioimage-analysis ! I left a couple of small comments about the tutorial. About the algorithm itself, I had a quick look at the paper by Fish et al. A comment in the code says that the implementation is not exactly what's in the paper because it does not work, could you please elaborate on this? Also, what is the message of the tutorial about the ideal number of iterations? I tried reducing the intensity of the noise and even with a very small noise the loss function was never monotonous (whereas it is in the paper for a low noise intensity), what do you think about this? |
I revisited the code and honestly, I think the code is not fully correct. And this is why "the hack" is needed. Everything actually refers to this part of the paper: However, there should be Here I programed it from scratch, reconstructing a cross similar to the paper:
And this is what is now (!) happening: Thus, I think the code is
|
Description
Follow up on @Anti-Xyz pull request #3524, added most of the modifications.
Checklist
./doc/examples
(new features only)./benchmarks
, if your changes aren't covered by anexisting benchmark
For reviewers
later.
__init__.py
.doc/release/release_dev.rst
.