Skip to content

Conversation

@gselzer
Copy link
Member

@gselzer gselzer commented Jan 9, 2024

This work is a quick cleanup of a stale branch adding a uniform noise Op, as well as a simple regression test. Some points on design decisions:

  • Unlike the design implemented in Add uniform noise op imagej/imagej-ops#554, this Op runs on RandomAccessibleIntervals and makes use of the LoopBuilder construct. For simplicity, computation is performed sequentially - I didn't see a clear pathway to run this in a multithreaded way, because we'd have to (a) use a thread-safe RNG object, which would be slower, and I didn't bother figuring out whether it would be worth it, or (b) create a MersenneTwisterFast per chunk, which is tricky because as far as I can tell Chunks give you no discernible state that we could use to seed each MersenneTwisterFast differently.
  • This Op takes no stance on overflow behavior - it's left up to the RealType how out-of-bounds values are handled. There are also situations where we may have lossy behavior - for example, if the range provided is very large - but I think that's a hard problem to solve anyways.

I honestly think this is a pretty low-priority solve - but at the very least, we've moved broken code from a stale branch to a PR that could be closed without losing anything important.

@gselzer gselzer added the enhancement New feature or request label Jan 9, 2024
@gselzer gselzer requested a review from hinerm January 9, 2024 21:37
@gselzer gselzer self-assigned this Jan 9, 2024
@gselzer gselzer force-pushed the stale/imagej/imagej-ops2/noise-multithreading branch from 0193aed to e9cbbfc Compare January 12, 2024 15:58
@hinerm hinerm force-pushed the stale/imagej/imagej-ops2/noise-multithreading branch from e9cbbfc to 684c0e0 Compare January 16, 2024 15:21
@hinerm hinerm merged commit 434ae65 into main Jan 16, 2024
@hinerm hinerm deleted the stale/imagej/imagej-ops2/noise-multithreading branch January 16, 2024 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

3 participants