tensorics / tensorics-core

The core library of Tensorics - a Java Library for Manipulating Multi-Dimensional Data with Pleasure
http://tensorics.org
Apache License 2.0
10 stars 2 forks source link

java.lang.IllegalArgumentException: Multiple entries with same key in an empty bi-dimensional tensor #4

Closed Cs4r closed 8 years ago

Cs4r commented 8 years ago

I came across with some unexpected behaviour when creating a tensor with two dimensions, both of them having the same class.

In essence I wanted to perform the addition of two tensors that do not share common coordinates. I attach the code that illustrates the problem and the exception that it throws at line 6 -> builder2.putAt(1D, 0, 1); .

Reading Coordinates.mapOf method I deduced that I cannot have duplicate keys (dimensions) but I was able to create the first tensor (tensor1) indeed.

Snippet:

TensorBuilder<Double> builder1 = Tensorics.builder(Integer.class, Integer.class);
builder1.putAt(1D, 0, 0);
builder1.putAt(1D, 1, 1);
Tensor<Double> tensor1 = builder1.build();

TensorBuilder<Double> builder2 = Tensorics.builder(Integer.class, Integer.class);
builder2.putAt(1D, 0, 1);
builder2.putAt(1D, 1, 0);
Tensor<Double> tensor2 = builder2.build();

Tensor<Double> minus = calculate(tensor1).plus(tensor2);
System.out.println(minus);

Stack trace:

java.lang.IllegalArgumentException: Multiple entries with same key: class java.lang.Integer=1 and class java.lang.Integer=0
    at com.google.common.collect.ImmutableMap.checkNoConflict(ImmutableMap.java:150)
    at com.google.common.collect.RegularImmutableMap.checkNoConflictInBucket(RegularImmutableMap.java:104)
    at com.google.common.collect.RegularImmutableMap.<init>(RegularImmutableMap.java:70)
    at com.google.common.collect.ImmutableMap$Builder.build(ImmutableMap.java:254)
    at com.google.common.collect.ImmutableClassToInstanceMap$Builder.build(ImmutableClassToInstanceMap.java:107)
    at org.tensorics.core.tensor.Coordinates.mapOf(Coordinates.java:59)
    at org.tensorics.core.tensor.Position.<init>(Position.java:56)
    at org.tensorics.core.tensor.Position.of(Position.java:69)
    at org.tensorics.core.tensor.Position.of(Position.java:79)
    at org.tensorics.core.tensor.AbstractTensorBuilder.putAt(AbstractTensorBuilder.java:85)
    at org.tensorics.core.examples.sprint.MyTest.myTest(MyTest.java:23)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:497)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
    at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:459)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:675)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:382)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:192)
kaifox commented 8 years ago

Hi,

whow! Thanks a lot for spotting this! This is really a subtile bug, completely non-understandible for a user! I believe, the problem comes from the fact that in the builder, as well as in the position we convert the dimesions (and the cooridinates in the position) from the arrays of the varargs to sets(!). So in your first builder, the coordinates were exactly the same, so the set contained only one element. So what actually happened is that you created a one-dimensional tensor, not a two dimensional one. Completely non-transparent, I agree.

In the second builder, the positions were different, so the set contained two elements, while the builder had again one dimension.... so the exception is correctly thrown.

The behaviour should be: Already throw in the constructor of the builder, if there are two equal classes (later probably if the classes are not 'disjunct' in class hierarchy). So the check has to be done before converting to the set. (It might also be discussed if a set is a good structure for the other mehtod, however, since the set is in the method signature, it should be clear for a user.

The same check has to be done in the position before converting to a set (Again, afterwards, a set should be ok, because since the elements cannot be of the same class, they never should be equal (assuming a correct implementation of the equals method), thus staying the same when converting from array to set.

kaifox commented 8 years ago

This issue should be treated with my previous commit. Now it should give better and more strict exceptions. Any further checks from your side are more than welcome! (I close the issue for the moment)