Add uniform noise op

[gselzer]

This PR adds an op to add uniformly distributed noise throughout an image, as an alternative to the non-uniform AddGaussianNoise op, renamed in this PR from AddNoise.

[ctrueden]

When working with ImgLib2 types (such as UnsignedByteType), this is the observed behavior, and it actually comes in handy when working with cyclic colormaps

I suggest caution: there is no guarantee that all ImgLib2 types will behave in this way. (@tpietzsch @axtimwalde please correct me if I am wrong. ) Behavior in this case is left up to the implementation. E.g., IIRC, there are some types that when you setReal(value) them with an out-of-bounds value, they get corrupted somehow. @awalter17 do you remember?

[awalter17]

there are some types that when you setReal(value) them with an out-of-bounds value, they get corrupted somehow. @awalter17 do you remember?

I don't remember a problem with setReal (sorry!), but there was inconsistency with the out-of-bounds behavior (some wrapped, others clamped). I think most of them have masking/wrapping behavior now (https://github.com/imglib/imglib2/pull/96 - this was for constructors but I think it affected the setters as well). That being said, I don't think RealType guarantees this wrapping behavior for setReal or any of the setters.

[imagejan]

I suggest caution: there is no guarantee that all ImgLib2 types will behave in this way.

Yes, I figured it would be undefined behavior (as always in ImgLib2, it's the callers responsibility to conform to types and intervals...), that's why i said "observed" behavior :smile: Ok, I will not rely on that, but the "cyclic scale" use case came to my mind as something where this would be really useful.

when you setReal(value) them with an out-of-bounds value

BTW, are there terms to distinguish between out-of-bounds for intervals (i.e. x,y,z,...) and out-of-bounds for types (i.e. values)?

Anyhow, this is now far off-topic for this PR. I suggest to simply ignore my comments (and maybe just keep in mind that it would be useful to have consistent clipping/wrapping behavior across ops in general).

[gselzer]

Alright, I gave this PR some TLC. @imagejan, after some thinking I realized that I liked your solution a bit better, as I described in 5440061. I found it difficult to treat floating point and integer types the same, however (see 04af3fe), so I created two different Ops.

I did, however, leave the computations for the integer type Op in doubles, however, to allow for some support for some of our larger IntegerTypes. The only other option that I saw was to convert everything to BigIntegers, but I did not want to do that for performance reasons. (Note that we cannot simply do this all using Imglib math because that would leave us open to all of the issues of the edge behavior of each type) Let me know what you think.

[gselzer]

I think the way setting the seed parameter works should be flipped, i.e. don't use a constant seed unless otherwise specified.

Thanks for catching that @rimadoma! Should be updated in the uniform noise Ops. It looks like AddGaussianNoise and AddPoissonNoise do this as well, should we alter them with the risk of breaking backwards compatibility?

Also Random is not thread safe.

Sure, yes. I used MersenneTwisterFast at @ctrueden's recommendation (which is thread safe according to the Javadoc). We can switch this in the other noise Ops as well if we decide it is okay to do so.

[ctrueden]

The previous default seed parameter was correct. One rule of ops is that they are deterministic. If you want TRULY RANDOM noise in your script, then you must generate a random number (externally to ops) and pass that as the seed. I know it's annoying, but it's one of the guarantees of ops.

Therefore, we also need to be careful about multithreaded pseudorandomness. Just because MersenneTwisterFast is thread-safe doesn't mean the random order will be deterministic per thread. You need to use a separate instance of the RNG per thread. Happy to discuss more tomorrow during our meeting, @gselzer.