JuliaImages / Images.jl

An image library for Julia
http://juliaimages.org/
Other
525 stars 141 forks source link

reimplementation of `imgaussiannoise` #745

Open johnnychen94 opened 5 years ago

johnnychen94 commented 5 years ago

This is not an urgent issue since it could be fairly easy to implement by users themselves, which is the common case as far as I know.

  1. Currently, I find only imgaussiannoise, but there are noises such as salt & pepper, possion, speckle. So I think we could expose an imnoise wrapper to make it convenient for users who want to add/generate whatever noise to an image.

It can be things like this,

function imnoise(img::AbstractArray{T}, method::String, args...)
    if method == "gaussian"
        return imgaussiannoise(img,args...)
    elseif method == "salt & pepper"
        return imsaltandpepper(img,args...)
    else
        ArgumentError("$method not supported")
    end
end
  1. As for imgaussiannoise

    • there's a small bug on imgaussiannoise, since a::AbstractArray + b::Number now raises an MethodError in julia v1.0.0

    • Another potential issue about this comes with its return value type, should it be T or Float64? The current version returns Array{Float64,1} for imgaussiannoise(colorview(Gray,n0f8.(zeros(5)))). In my opinion, at least it should not change the type of img without warning.

    • And it has to deal with the "annoying" N0f8 overflow behavior. Using clamp would be okay for MATLAB users, but I'm not sure if this is okay for others.

function Images.imgaussiannoise(img::AbstractArray{T}, variance::Number, mean::Number)::AbstractArray{T} where T
    return clamp01.(img + sqrt(variance)*randn(size(img)) .+ mean)
end
  1. and there's no test cases on imgaussiannoise method.
johnnychen94 commented 5 years ago

I'm going to implement a more general imnoise function than the current imgaussiannoise in the following way

abstract type Noise end
struct GaussianNoise{T} <: Noise
    mean::T
    std::T
end
GaussianNoise(mean::S, std::T) where {S,T} = GaussianNoise{promote_type(S,T)}(mean, std)

imnoise(::AbstractArray, noise::Noise) 
    = error("imnoise for $(typeof(noise)) type is not supported yet.")
imnoise(img::AbstractArray, noise::GaussianNoise) 
    = img + noise.std .* randn(eltype(img), size(img)) + mean .* ones(eltype(img),size(img))

Ask:

Tokazama commented 5 years ago

It might make sense to use the Distributions package so you don't have to reinvent everything.

timholy commented 5 years ago

In general I'm leaning away from the im* naming scheme: images are just arrays, so why should functions have im in front of them?

More importantly, we should think about what this really does, and whether it offers much value beyond a comprehension [val + generator() for val in img]. I guess in some circumstances it's faster to generate all the random numbers at once, so that might be justification in and of itself.

Tokazama commented 5 years ago

I think there's merit to it if it's part of a larger project incorporating similar things though. I've been toying with the idea of a package like ImageStats, but haven't had the time to put together enough coherent ideas to constitute a pull request or and independent package. I'd be happy to contribute to such an effort if someone beats me to it.

As for the first point, I think it'd be nice to have a contributors guide with some details for styling rules in the JuliaImages universe. Something more opinionated and specific than the style guide in the Julia docs.

johnnychen94 commented 5 years ago

In general I'm leaning away from the im* naming scheme: images are just arrays, so why should functions have im in front of them?

Personally, I think it makes the function too generic to remove im prefix, for instance, if changed imresize to resize(::AbstractArray), it would possibly conflict with the same method resize(::AbstractArray) from other packages. One rule of thumb I can think of is to

zygmuntszpak commented 5 years ago

I would also prefer to avoid im prefix in function names. How about something like:

abstract type AbstractNoise end
struct GaussianNoise <: AbstractNoise end
struct PoissonNoise <: AbstractNoise end

With function names

apply_noise(::GaussianNoise, ...)
apply_noise(::PoissonNoise, ...)
timholy commented 5 years ago

Let's move general discussion about naming conventions to #767