jeanluct / braidlab

Matlab package for analyzing data using braids
GNU General Public License v3.0
24 stars 10 forks source link

Array display #30

Closed jeanluct closed 9 years ago

jeanluct commented 9 years ago

When an empty array of braids is created, Matlab fails in 'disp' method of the braid as it immediately tries to access .word property, even though an array has been passed to it.

E.g.

a(1,10) = braid

fails.

jeanluct commented 9 years ago

From Marko Budisic on 2014-01-09 19:06:53+00:00

Arrays of braids are now correctly displayed using disp(). Resolve Issue #30.

→ <<cset 09a357cd5c4e>>

jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-01-09 21:26:01+00:00

Wait, should we be able to make arrays of braids? I would have thought only cell arrays would be allowed.

jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-01-09 21:27:07+00:00

Also, please follow the braidlab formatting convention: 2 spaces indent (not 4).

jeanluct commented 9 years ago

From Marko Budisic on 2014-01-09 21:28:11+00:00

IDK but we're able to. I just tried it, it works just fine once I fixed the disp() bug.

jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-01-09 21:34:36+00:00

Have you tested a few vectorized operations? How does multiplication, etc work? Can you act with a vector of braids on a vector of loops? loops have been written explicitly to vectorize internally, so I don't see how this could work.

jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-01-09 21:35:31+00:00

Not sure we should allow this... might have confusing side effects. At the very least we should do some testing, and add vectorized examples to the testsuite.

jeanluct commented 9 years ago

From Marko Budisic on 2014-01-09 21:40:41+00:00

No, the operations don't work, although if we wanted, we could make them work. Even so the arrays of braids are useful, for example when you're parallelizing the code using parfor. If you have, say, 10 groups of 3 trajectories of which each should be converted to a braid, you can loop through groups using parfor, in which case each braid is computed on a separate processor code. For Matlab to be able to parallelize, you need to specify the output structure. In this case, array of braids works well.

A cell array would be OK too, but just a pure braid array makes sense here, no? Not everything that Matlab can store in braids has to have a good vectorized set of operations associated to it, e.g., characters/strings.

jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-01-09 21:53:41+00:00

Except it's the opposite issue here: if you make an array of braids then you might expect some "natural" behavior.

I'm not saying it's not a good idea, but I think it bears thinking about carefully before implementing. For now my thought had always been that braids were more naturally cells. For instance, it doesn't make sense to have an array of arrays which are different lengths, and braids could have different lengths.

Doesn't what you describe (output structure) work better as a cell array?

jeanluct commented 9 years ago

From Marko Budisic on 2014-01-09 22:01:54+00:00

I don't know, it might. The largest difference as far as I can see is in initialization. For example, setting

a(10) = braid(1),

creates identity braids for a(1:9), and initializes a(10) to <1>.

a{10} = braid(1),

creates empty objects for a(1:9).

Frankly, I'm agnostic to how we could solve it. Cell array works in my code, however, prohibiting creation of object arrays might be difficult, from the Matlab standpoint, as what Matlab does is just calls an empty constructor everywhere in between (1) and initialized object. Having the code fail on the disp() function if one tries to create a braid array is a bit clunky.

The rev:309 code would succeed if one issued

a(10) = braid; % semicolon

but fail if one issued

a(10) = braid % no semicolon

since in the second case, disp is implicitly called.

jeanluct commented 9 years ago

From Marko Budisic on 2014-01-09 22:08:04+00:00

I created Issue #32 to continue this discussion, as Issue #30 dealt only with the disp() function.

jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-02-15 21:16:05+00:00

Ok, in b8e8d401 I now derive all classes from matlab.mixin.CustomDisplay. Then instead of providing disp, we specialize the protected method displayScalarObject, and optionally displayNonScalarObject (for loops). If displayNonScalarObject is not provided, then the display defaults to the "right thing":

#!matlab

>> b(1,10)=braid
b = 
  1x10 braid array with properties:

    n
    word

without having to do anything. In the class loop I specialize that method:

#!matlab

>> l=loop([1 2; 3 4])
l = 
   (( 1  2 ))
   (( 3  4 ))

since arrays of loops are more common. Note that this also works with

#!matlab

>> l(1,5) = loop
l = 
   (( 0 -1 ))
   (( 0 -1 ))
   (( 0 -1 ))
   (( 0 -1 ))
   (( 0 -1 ))
jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-02-15 21:16:40+00:00

See b8e8d401