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

Sparse bitmask #44

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open

Sparse bitmask #44

wants to merge 27 commits into from

Conversation

tpietzsch
Copy link
Member

SparseBitmask is a RandomAccessible<BoolType> based on nTree (quadtree, octree, ...) with little bitmasks (basically ArrayImg<BitType>) in the leafs. It's a RandomAccessible (not Interval). The tree grows on demand, just set pixels wherever...

There are efficient algorithms for

  • computation of bounding box of set (true) pixels,
  • iteration of set (true) pixels.

These are provided through a (read-only) view as an IterableRegion.

Some usage examples are in src/test/.

sparsetree

I'm happy with the underlying tree and bitmask stuff (SparseBitmaskNTree, Tree, GrowableTree).
However, packaging as a RandomAccessible in SparseBitmask required some trade-offs that are not so clear and depend on use case. I would be happy about some input...

In particular, read/write of pixels is protected by a ReentrantReadWriteLock that is acquired per pixel access. It would be nice to have this more coarse-grained, but I don't know how.

Also, SparseBitmask.region() provides IterableRegion view of the current state, and will throw ConcurrentModificationException when the bitmask is modified. There are some minor trade-offs there as well, but read/write locking is the major problem.

The tree is truncated such that it does not contain boolean value-nodes
all the way to the leafs. Instead, leafs store bitmasks of a specified
size.

Also contains algorithms to efficiently compute the number and bounding box
of set pixels in such bitmasks.
A RandomAccessible<NativeBoolType> that grows on demand.
SparseBitmask.tree() provides the underlying SparseBitmaskNTree.
SparseBitmask.region() provides a view as IterableRegion, with efficient
computation of bounding box and iteration of set pixels.
@@ -197,5 +197,10 @@ Jean-Yves Tinevez and Michael Zinsmaier.</license.copyrightOwners>
<artifactId>junit-benchmarks</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>sc.fiji</groupId>
<artifactId>bigdataviewer-vistools</artifactId>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Maven build on Travis now fails because bigdataviewer-vistools contains a transient dependency on imglib2-roi, creating a circular dependency.

As this is a test scope dependency only, the issue can be avoided by adding an exclusion:

		<dependency>
			<groupId>sc.fiji</groupId>
			<artifactId>bigdataviewer-vistools</artifactId>
			<scope>test</scope>
			<exclusions>
				<exclusion>
					<groupId>net.imglib2</groupId>
					<artifactId>imglib2-roi</artifactId>
				</exclusion>
			</exclusions>
		</dependency>

I'm not sure though if it is the best way to address this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting hack—I didn't know that worked.Personally, I'd vote for the usage examples requiring vistools to move downstream. Perhaps into vistools itself, since it already depends on imglib2-roi transitively.

@maarzt
Copy link
Contributor

maarzt commented Jan 9, 2019

@tpietzsch I made some quick changes.

  1. Bitmask can be a separate class. That should make the class Tree more readable.
  2. There's a bug with the SpareIterable.region().cursor(), if there one bit set, it iterates over that pixel twice? So there probably a small bug in hasNext.

maarzt and others added 3 commits January 10, 2019 00:15
The functionality of Bitmask is very much independent of the class Tree.
It should be an independent class.
Only on bit of SparseBitmask is true.
But SparseBitmask.region().cursor() seems to have two elements?
@tpietzsch
Copy link
Member Author

@maarzt I fixed the bug. Thanks for noticing!

@maarzt maarzt self-requested a review January 10, 2019 10:32
@maarzt
Copy link
Contributor

maarzt commented Jan 10, 2019

Bitmask is probably not a got term to refer to a black/white image. Mask or Bitmap are preferable. Bitmask is rather one dimensional.
https://en.wikipedia.org/wiki/Bitmap
https://en.wikipedia.org/wiki/Mask

* numSet -> numberOfOnes (numSet is too ambiguous. Is it a set of numbers?)
* leafDims -> dims
* bytes -> data (The javadoc comment suggests this renaming.)

Add method byteMask similar to byteIndex to reduce code duplication.
The design of this class is specific to it's use in Tree this
should be reflected by the name
The sparse bitmaps in Labkit use list's of coordinates to store only the
white pixels.
If a set operation sets NodeData.children or NodeData.bitmask to zero,
get will return NodeData.value.
That way GrowableTree and Tree can be better distinguished.
Also reading Tree becomes thread-safe. The tree can always be red in any
thread, only the write operations need to be synchronized.
@imagesc-bot
Copy link

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/imglib2-imaris-bridge-sharing-cached-images-between-imaris-and-fiji/57772/11

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.

5 participants