PiRSquared17 / alembic

Automatically exported from code.google.com/p/alembic
Other
0 stars 0 forks source link

Dimensions may not be preserved when sharing the same data. #139

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1.  Write 2 datasets with the same layout (all 0s) one that is 3x2x4 and one 
that is 4x3x2.

2.  Read the data, whatever one was written first will set the dimensions for 
both.

What is the expected output? What do you see instead?
The dimensions should be 3x2x4 for one attribute and 4x3x2 for the other.

The problem is that the "dims" info is an attribute that belongs to the dataset.
That data will need to be moved up to the property group and stored per sample.

Original issue reported on code.google.com by miller.lucas on 18 Feb 2011 at 1:16

GoogleCodeExporter commented 9 years ago
Since ArraySample also stores the Dimensions, I'm not sure we can fix this 
totally without an API change.

This contains some fixes on where the data is stored, but the reader can't pick 
it up:
http://code.google.com/r/millerlucas-dev/source/detail?r=a24cd22e18f60fc840a9e07
96c840e36fc5bc628

And the commit before, contains a simple test for this:
http://code.google.com/r/millerlucas-dev/source/detail?r=5b645bda45423f189e190ba
e09a247637173fc04

The good news is that I really can't see AbcGeom ever needing Dimensions.

Original comment by miller.lucas on 2 Mar 2011 at 2:29

GoogleCodeExporter commented 9 years ago
I found another issue where datasets that were shared but have different 
extents could not be read back in.

This sums up all of the changes so far:
http://codereview.appspot.com/4253053/

Note: The problematic Dimensions test is currently commented out until we can 
discuss it.

Original comment by miller.lucas on 3 Mar 2011 at 9:38

GoogleCodeExporter commented 9 years ago
Hey Lucas, can you confirm if this issue is fixed or not?

Original comment by ard...@gmail.com on 16 May 2011 at 10:41

GoogleCodeExporter commented 9 years ago
I believe it's correct on write, but incorrect when reading with caching on.

Original comment by miller.lucas on 16 May 2011 at 10:47

GoogleCodeExporter commented 9 years ago
Ah.

So I guess this is still not working?

Original comment by ard...@gmail.com on 18 May 2011 at 7:25

GoogleCodeExporter commented 9 years ago
Yes, this bug still exists and will most likely require an API change to fix.

Original comment by miller.lucas on 18 May 2011 at 7:41

GoogleCodeExporter commented 9 years ago

Original comment by miller.lucas on 15 Jun 2011 at 5:50

GoogleCodeExporter commented 9 years ago
The workaround is here:
http://code.google.com/r/millerlucas-dev/source/detail?r=a1a0ec0cdfbffaca638818f
ab5ea9ec9f41bc075

The dimensions advertised by the ArraySample can still be wrong when caching is 
on and more than one dataset points at the same data, but for apps where it is 
critical that they get the correct dimensions (the data layout will still be 
flat so the data in ArraySample is always correct) they can call getDimensions.

While not ideal, I feel this is better than bloating ALL of our SampleKeys with 
Dimensions, and this data should be shared anyway.

Original comment by miller.lucas on 22 Jun 2011 at 10:42

GoogleCodeExporter commented 9 years ago
OK, I've brought in your changes, and verify the tests work correctly:

http://code.google.com/r/ardent-embic/source/detail?r=1a2aa403f7d761f5dcf92a2e38
556edd8e53ac31&name=default

You should sync your AbcCoreHDF5 directory with mine; there are a few things 
you're missing, such as 
http://code.google.com/r/ardent-embic/source/detail?r=4699ff50e74ab1a45a733ab8ce
8fae4a72135e67&name=default .  You might want to just use your favorite 
directory diffing tool to check out our respective Abc, AbcGeom, and 
AbcCoreHDF5 dirs.

Original comment by ard...@gmail.com on 23 Jun 2011 at 12:10

GoogleCodeExporter commented 9 years ago
I haven't integrated the mutex changes on purpose, I still haven't measured 
it's cost as far as memory and read speeds are concerned.

Original comment by miller.lucas on 23 Jun 2011 at 12:53