Skip to content
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 BM3D(sparse 3D transform-domain collaborative filtering) denoising algorithm. #21

Merged
merged 9 commits into from
Aug 4, 2021

Conversation

Longhao-Chen
Copy link
Contributor

No description provided.

@johnnychen94
Copy link
Member

johnnychen94 commented Aug 3, 2021

Thanks! Any reason to make it a conditional dependency? (FYI, usually, we use https:/JuliaPackaging/Requires.jl for conditional codes)

Can you also add a demo similar to https:/JuliaImages/ImageNoise.jl/blob/master/docs/examples/reduce_noise/nonlocal.jl?

Also, it would be great to also have a regression test to make sure future improvement don't make it worse, like how

@testset "Numeric" begin
img_gray = n0f8.(imresize(testimage("lena_gray_256"); ratio=0.25))
n = AdditiveWhiteGaussianNoise(0.05)
noisy_img = apply_noise(img_gray, n; rng=MersenneTwister(0))
f = NonlocalMean(0.05, 2)
denoised_img = reduce_noise(noisy_img, f)
# further modification shall not decrease psnr and ssim
assess(PSNR(), denoised_img, img_gray) >= 28.46
assess(SSIM(), denoised_img, img_gray) >= 0.918
end
does.

@Longhao-Chen
Copy link
Contributor Author

Could you tell me what the role of conditional dependency is?

@johnnychen94
Copy link
Member

johnnychen94 commented Aug 4, 2021

  • hard dependency: directly add BM3DDenoise to Project.toml and let Pkg to manage it
  • conditional dependency: don't let Project.toml manage it, and only load it under some circumstance. In the current version, BM3DDenoise will only be loaded when reduce_noise(img, BM3D()) is called.

Generally, we should use hard dependency and let Pkg do everything for us, because it allows 1) less code management, and 2) precompilation.

In some typical cases like FileIO there are too many possible IO backends, we use conditional loading to 1) don't run into a gigantic Project.toml because not everyone needs backend support for every file format, 2) reduce loading time

There're some discussions in JuliaLang/Pkg.jl#977 to allow Pkg do conditional dependency management.


In our case, I think we can just use the hard dependency as if BM3DNoise is a normal package.

@Longhao-Chen
Copy link
Contributor Author

I don't think everyone will use this function. For example, if I just want to use apply_ noise, then I don't have to install BM3DDenoise.jl , which can save disk space. Just like FileIO .
As for precompiling, I did an experiment. Precompiling will be performed during BM3DDenoise.jl installation.

@johnnychen94
Copy link
Member

johnnychen94 commented Aug 4, 2021

I realize that this package is a bit old and broken, so I've made a PR in #22 to upgrade it. (Just merged)

I don't think everyone will use this function.

Okay, in this case, we can use Requires.jl for this purpose. We can write BM3DDenoise.jl in the usual way, and then conditionally load this file using Requires.

    # in src/ReduceNoise/ReduceNoise.jl
    using Requires

    function __init__()
        @require BM3DDenoise="95fb3b36-088a-43fb-bb1b-b1f34fadbd7d" include("BM3DDenoise.jl")
    end

@johnnychen94
Copy link
Member

Oh sorry! Forget about the Requires stuff. I didn't get your ideas correctly. This is a nice idea and I think we should propose to other packages as well.

@johnnychen94
Copy link
Member

Let's just add some basic demo and basic tests. And add BM3DDenoise to Project.toml

@Longhao-Chen
Copy link
Contributor Author

Longhao-Chen commented Aug 4, 2021

Let's just add some basic demo and basic tests. And add BM3DDenoise to Project.toml

OK, I will.

@codecov
Copy link

codecov bot commented Aug 4, 2021

Codecov Report

Merging #21 (c7f9cae) into master (2cf0300) will increase coverage by 0.61%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #21      +/-   ##
==========================================
+ Coverage   93.61%   94.23%   +0.61%     
==========================================
  Files           5        6       +1     
  Lines          94      104      +10     
==========================================
+ Hits           88       98      +10     
  Misses          6        6              
Impacted Files Coverage Δ
src/ReduceNoise/BM3DDenoise.jl 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2cf0300...c7f9cae. Read the comment docs.

docs/examples/reduce_noise/BM3D.jl Outdated Show resolved Hide resolved
src/ReduceNoise/BM3DDenoise.jl Outdated Show resolved Hide resolved
Project.toml Outdated Show resolved Hide resolved
@johnnychen94 johnnychen94 merged commit f662676 into JuliaImages:master Aug 4, 2021
@johnnychen94
Copy link
Member

@Longhao-Chen thank you for adding this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants