discoverable axes index lookup #14

axtimwalde · 2019-12-04T19:22:57Z

add discoverable axes index lookup method as proposed by @d-v-b
rename N5Utils to N5 and deprecate N5Utils

* renamed N5Utils to N5 and deprecated N5Utils

axtimwalde · 2019-12-04T20:38:09Z

@hanslovsky @igorpisarev, I mainly want you to familiarize with this before making it public so we can start using it immediately. Of course, if you find any shortcomings, please let me know. Thanks!

hanslovsky · 2019-12-05T15:21:52Z

doc/axes.md

+│
+├── ...
+┊
+```


I don't really like that .axes is in the root of the container. This makes datasets less portable (i.e. I can't just copy the dataset itself but I will have to remember to also copy the appropriate dataset in .axes. Also, what if a dataset with the same name already exists in .axes of the target container?

We could also store .axes inside the actual dataset, e.g.

├── "dataset" │ ├── "attributes.json" │ ├── ".axes" │ │ ├── "attributes.json" That would make datasets more portable and you would not need any global container information to copy a dataset.

then we cannot use the dataset loader to resolve the ordering because there cannot be a dataset in a dataset (e.g. in HDF5). I agree about it making portability harder which is only an issue if this feature is used which it currently isn't.

hanslovsky · 2019-12-05T15:22:01Z

doc/axes.md

+```
+-1  0  ->  -1 0 1 -1
+ 1 -1
+```


Is there a more detailed specification of what

-1 0 1 -1

and

-1 0 1 -1

mean? I have a hard time understanding how they are generated.

Looking at the source code helps a lot (thanks @igorpisarev for pointing out the correct names), so maybe doc/axes.md could be more specific about what exactly is happening to create tensors like

-1 0 1 -1

Agree but currenty have no better idea. Will work on it.

hanslovsky · 2019-12-05T15:24:58Z

doc/axes.md

+
+The ImgLib2 bindings offer API methods to create and use such lookups.  Naturally, the mapping is defined as coming from ImgLib2 F-order:
+
+```java


I could not find createAxes or any of the readVectorAttribute/writeVectorAttribute methods in n5-imglib2. Are they supposed to be part of n5-imglib2 or do they live in n5?

$ ag 'createAxes' doc/axes.md 40:createAxes( $ ag '(read|write).*Vect' doc/axes.md 44:readDoubleVectorAttribute( 49:writeDoubleVectorAttribute( 55:readLongVectorAttribute( 60:writeLongVectorAttribute(

It seems that the actual naming is a bit different, and they are called setAxes(), getAxes(), getDoubleVector(), setVector() in the code

Thanks @igorpisarev

Point taken.

hanslovsky · 2019-12-05T15:28:06Z

src/test/java/org/janelia/saalfeldlab/n5/imglib2/N5Test.java

+
+public class N5Test {
+
+	static private String testDirPath = System.getProperty("user.home") + "/tmp/n5-imglib2-test";


Why not simply use Files.createTempDirectory?

Good point, Tobias had a fix for that earlier and I will find out where that was and adjust.

hanslovsky · 2019-12-05T15:45:14Z

src/test/java/org/janelia/saalfeldlab/n5/imglib2/N5Test.java

+	private static final String EMPTY_DATASET = "/test/group/empty-dataset";
+
+	private static final int EMPTY_BLOCK_VALUE = 123;
+


Order of keywords is inconsistent: static private vs private static. I suggest we follow this order

public protected private static final transient volatile

as is the recommendation of the Java docs:

If two or more (distinct) field modifiers appear in a field declaration, it is customary, though not required, that they appear in the order consistent with that shown above in the production for FieldModifier.

Similarly, for method modifiers and class modifiers

Agree, I also want to change the formatter to saalfeldlab instead of imglib2, will do in a separate commit.

igorpisarev · 2019-12-05T17:03:11Z

I understand the way how n5-imglib2 permutes the vectors back and forth, but how would for example N5 HDF5 and Python N5 API interoperate with it? Do they need to account for their inverse access order (with respect to n5-imglib2)? Such as saving all vector attributes with the axes lookup [2,1,0], and applying the inverse permutation when reading them on top of the stored lookup.

Also, my understanding is that the axes lookup is not particularly imglib2-specific and can be also supported by other N5 implementations later down the road (Python, Rust). Would it make sense then to move the axes lookup support into the main N5 repository?

igorpisarev · 2019-12-05T17:06:56Z

src/test/java/org/janelia/saalfeldlab/n5/imglib2/N5UtilsTest.java

- * Copyright (c) 2017-2018, Stephan Saalfeld, Philipp Hanslovsky, Igor Pisarev
+ * Copyright (c) 2017-2019, Stephan Saalfeld, Philipp Hanslovsky, Igor Pisarev
+ * John Bogovic


If we're deprecating N5Utils, this file probably needs to be marked as deprecated as well

igorpisarev · 2019-12-05T17:14:04Z

src/main/java/org/janelia/saalfeldlab/n5/imglib2/N5.java

+		for (int s = 0; s < numScales; ++s) {
+			final String datasetName = group + "/s" + s;
+			final long[] dimensions = n5.getAttribute(datasetName, "dimensions", long[].class);
+			final long[] downsamplingFactors = n5.getAttribute(datasetName, "downsamplingFactors", long[].class);


Do we need to try to read downsamplingFactors as a vector attribute here? (and similarly in other places, it seems that there are more vector-like attributes in N5DisplacementField)

Good point. This can come in later as we start using the feature more.

igorpisarev · 2019-12-05T17:17:05Z

Another a bit confusing thing to me about interoperability: do we always assume that the essential attributes dimensions and blockSize are always stored in F-order, such as [x,y,z]? Since they're basically vector attributes too, I would expect them to be written and read via the same vector attribute API that supports axes lookup.

hanslovsky · 2019-12-05T21:04:41Z

I think one way around the issue of axes stored in /.axes vs /path/to/dataset/.axes could be resolved by creating a method that simple creates an axes dataset with an arbitrary name. This method could then also be used to store the axes in /.axes. In other words, n5-imglib2 would only be concerned with storing axes as a dataset but would not impose a convention at all of where these axes should be located.

axtimwalde · 2020-05-26T22:44:01Z

Python, C++ or Rust can find out by loading the axes volume, that 's the whole point of the exercise.
dimensions and blockSize are API attributes and are F-order by spec.

axtimwalde added 4 commits November 22, 2019 23:39

* add discoverable axes index lookup method as proposed by @d-v-b

d62daad

* renamed N5Utils to N5 and deprecated N5Utils

started axes documentation

daa1bf2

bump to pom-scijava-27.0.1, n5-2.1.3, imglib2-label-multisets-0.9.0

548ac11

add missing primitive vector types, fix small bugs, document

1c80fde

axtimwalde requested review from igorpisarev and hanslovsky December 4, 2019 19:23

use N5 instead of N5Utils in script example

9c11334

hanslovsky reviewed Dec 5, 2019

View reviewed changes

igorpisarev reviewed Dec 5, 2019

View reviewed changes

Merge branch 'master' into axes

c7d0606

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

discoverable axes index lookup #14

discoverable axes index lookup #14

axtimwalde commented Dec 4, 2019

axtimwalde commented Dec 4, 2019

hanslovsky Dec 5, 2019

axtimwalde May 26, 2020

hanslovsky Dec 5, 2019

hanslovsky Dec 5, 2019

axtimwalde May 26, 2020

hanslovsky Dec 5, 2019

igorpisarev Dec 5, 2019

hanslovsky Dec 5, 2019

axtimwalde May 26, 2020

hanslovsky Dec 5, 2019

axtimwalde May 26, 2020

hanslovsky Dec 5, 2019

axtimwalde May 26, 2020

igorpisarev commented Dec 5, 2019

igorpisarev Dec 5, 2019

igorpisarev Dec 5, 2019

axtimwalde May 26, 2020

igorpisarev commented Dec 5, 2019

hanslovsky commented Dec 5, 2019

axtimwalde commented May 26, 2020


		The ImgLib2 bindings offer API methods to create and use such lookups. Naturally, the mapping is defined as coming from ImgLib2 F-order:

		```java


		public class N5Test {

		static private String testDirPath = System.getProperty("user.home") + "/tmp/n5-imglib2-test";

		private static final String EMPTY_DATASET = "/test/group/empty-dataset";

		private static final int EMPTY_BLOCK_VALUE = 123;

discoverable axes index lookup #14

Are you sure you want to change the base?

discoverable axes index lookup #14

Conversation

axtimwalde commented Dec 4, 2019

axtimwalde commented Dec 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igorpisarev commented Dec 5, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igorpisarev commented Dec 5, 2019

hanslovsky commented Dec 5, 2019

axtimwalde commented May 26, 2020