jeanluct / braidlab

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

braid.braid constructs empty braids non-consistently #58

Closed jeanluct closed 9 years ago

jeanluct commented 9 years ago

When a trivial braid is constructed from a list of generators, e.g., braid([],3), its word is a matrix [].

When a trivial braid is constructed from a trivial trajectory, e.g., braid(cat(3, [0,0], [1,1]),5), its word is Empty 0x1 matrix.

This ultimately ended up confusing compact_helper and caused segfaults (see rev 585). I added code to safeguard compact_helper from this issue (see rev 586), but this is more of a braid.braid issue, as the same (trivial) braid has two different representations.

jeanluct commented 9 years ago

From Marko Budisic on 2014-06-24 23:17:14+00:00

braid.compact now compacts only braids that have wordlength longer than 1; additional tests withing C++ compact_helper introduced to ensure that empty string is not passed. See Issue #58

→ <<cset 6c634229faad>>

jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-06-25 15:53:18+00:00

This would be easy to change: line 206 of braid.m just sets word=[] when the first argument is []. Probably it would be best to do this in the set method, line 231.

But what's the best representation to use? I guess the only ambiguity is that [] is 0 by 0, but the trivial trajectory returns a 0 by 1 matrix.

What I did: added a line

#!matlab

        % Make sure the empty word is 0 by 0.
        if isempty(br.word), br.word = []; end

in braid.m after creating a braid from data (b4e1eb3). Let me know if this works for you. Reopen if not.

jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-06-25 15:53:27+00:00

Make sure empty braid has word size 0 by 0. Closes issue #58.

→ <>

jeanluct commented 9 years ago

From Marko Budisic on 2014-06-25 15:54:41+00:00

Yes, I think this is good, I prefer [] to empty matrices.

jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-06-25 15:56:19+00:00

Well, [] is an empty matrix:

#!matlab

a = []
a= double.empty(0,0)

are the same.

jeanluct commented 9 years ago

From Marko Budisic on 2014-06-25 15:58:59+00:00

OK, I misspoke, I prefer 0x0 matrices to 0x1 matrices.

jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-06-25 16:02:29+00:00

Slightly improved in 91bd0b3. set.word is a better place to do this check.

jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-06-25 16:04:23+00:00

Oops, wasn't quite right: need an int32 empty (dcd8fec).

jeanluct commented 9 years ago

For some reason, this wasn't closed automatically when migrating fro BitBucket.