openworm / org.geppetto.recording

Python project allowing to create a recording for Geppetto
http://www.geppetto.org/
Other
5 stars 0 forks source link

Add support for matrices #1

Closed tarelli closed 9 years ago

tarelli commented 10 years ago

See here https://github.com/openworm/OpenWorm/issues/208#issuecomment-42156148

jrieke commented 10 years ago

I think it would be best to split the add_value method into two methods: 1) add_single_value: stores the value as one item for one timestep (no matter how many dimensions it has, so matrices and lists of any kind can be handled as one value) 2) add_multiple_values: unfold the first dimension and store each unfolded item for one time step (e. g. a 1D-array becomes a time series of single values, a 4D-array becomes a time series of 3D-arrays) In my opinion that's more clean and easier to understand than having the current add_value method which takes either single or multiple values and an additional method add_matrix or something like that.

@tarelli What do you think?

tarelli commented 10 years ago

@jrieke, can you show me a snippet for how to use the two methods you propose? What is the advantage over having one method to handle all of them? Thx!

creator.add_value('a.b.c.d', 3, 'float_', 'mV', MetaType.STATE_VARIABLE)
creator.add_value('a.b.c.d', [1, 2], 'float_', 'mV', MetaType.STATE_VARIABLE)
creator.add_value('a.b.c.d', [[1, 2],[3,4]], 'float_', 'mV', MetaType.STATE_VARIABLE)
jrieke commented 10 years ago

@tarelli I think the main problem is if you want to add a single matrix for one time step, e. g. the 2x2 matrix [[1,2],[3,4]]. The intuitive way to do this would be to call (like in your last line)

creator.add_value('a.b.c.d', [[1, 2],[3,4]], 'float_', 'mV', MetaType.STATE_VARIABLE)

However, this does not add the matrix as one element but unfolds it and adds its components as two items for two successive time steps ([1,2] for time step 1; [3,4] for time step 2). The two methods (or a boolean parameter, alternatively) are in my opinion the cleanest way to prevent this misunderstanding.

tarelli commented 10 years ago

@jrieke I see now and agree, I would have then two methods as you suggest, add_value and add_values!

tarelli commented 10 years ago

On hold until the evaluation of NSDF is completed, see here https://github.com/subhacom/nsdf/tree/subhasis

jrieke commented 10 years ago

Implemented this now in the RecordingCreator class (see development branch). Instead of the 2 methods as described above, I left it at the single method add_values and rather inserted a parameter is_single_value. If this flag is set to True, the given values (regardless of how many dimensions they have) are stored as a single value for a single time step. If it is False (default), the items of an iterable of values are stored for successive timesteps. This makes usage a bit easier, because you only have to care about the parameter if you want to store matrices (which is probably rather rare...)

gidili commented 9 years ago

@tarelli @jrieke I added a unit test for matrices and it's passing but I am seeing something strange when I look at the recordings file with the HDFViewer.

If the matrices are:

Step 1: [[1, 2, 3], [4, 5, 6], [7, 8, 9]] Step 2: [[10, 11, 12], [13, 14, 15], [16, 17, 18]]

This is what's in the file according to the viewer: screenshot 2015-04-16 19 27 32

It looks like it's only putting there the first value of each row in the matrix.

Must be a bug in RecordingCreator.create().

gidili commented 9 years ago

This is where the values get set, as you can see the stuff is there as expected but then somehow only the first element of each row makes it into the file:

screenshot 2015-04-16 19 58 27

For now the workaround is to probably flatten matrices into a single array.

jrieke commented 9 years ago

@gidili As far as I remember, that is simply due to HDFView, which has pretty limited capabilities when visualizing multi dimensional data (the data itself should be there in the file). Let me look into that later and investigate.

jrieke commented 9 years ago

Ah yeah, got it: Click on the yellow arrows in HDFView (see below), and you will see the additional dimensions (the order is quite unintuitive, unfortunately, but it really is the same matrix as you entered ;). image (Step 1 and 2 are reversed in that image vs your example)

Also, the dimensions of the data are listed in the field below the table: image

gidili commented 9 years ago

@jrieke ah! Good catch, thanks for looking into this! I added a bunch of unit tests in the process :)

jrieke commented 9 years ago

@gidili Ok, great! I also just added a comment in the docstring for add_values to make users aware of that.