search

jeanluct / braidlab

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

Problems with colorbraiding and taffy (non-MEX) #117

Closed jeanluct closed 9 years ago

jeanluct commented 9 years ago

With BRAIDLAB_braid_nomex=1, taffy('4rods',pi/2) returns a braid without complaint:

>> taffy('4rods',pi/2)
ans =  < 3 -1 -3  2  3  1  2 >

This braid is reducible, which contradicts taffy('4rods') as in the guide. I'm pretty sure the latter is right, so this is a new bug.

(See also #116 for the MEX version)

jeanluct commented 9 years ago

git bisect gives the first commit where things start going wrong: 267c402. This is exactly when areEqual was introduced.

So some poking around leads to line 79 of cross2gen: when the problem happens, we have

K>> Xtraj1(1)

ans =

    -7.654042494670957e-17

K>> Xtraj2(1)

ans =

    -6.888638245203862e-17

Hence, the numbers are not equal according to eqfuzzy. But they really should be. In other words, eqfuzzy needs an absolute tolerance.

Maybe we should get rid of relative tolerance entirely, and instead have a sensible default, letting the user overwrite it with AbsTol or something.

mbudisic commented 9 years ago

I agree. I think it will save us some hair-pulling and it makes sense to offer a user a "physical" scale to tune. If coordinates are miles on the ocean, perhaps 0.1 is a sensible AbsTol. If coordinates are in cm, like taffy, then perhaps 1e-4 is a more sensible thing that can be discerned.

mbudisic commented 9 years ago

BTW That's crazy that git bisect exists and works. Such a timesaver.

mbudisic commented 9 years ago

Should I wait until you first code it up in M-file and I'll just replicate?

jeanluct commented 9 years ago

This type of automatic bisection is actually fairly old (probably was in CVS). It is a pretty cool feature.

We'll have to talk about a strategy for changing eqfuzzy.

mbudisic commented 9 years ago

Oh I was just looking at bisect and thought that it actually somehow figures things out automatically. But I guess, giving you sequence of test cases is good (quick) enough.

jeanluct commented 9 years ago

I think there are ways to automate it if you have a script that can test for a bug.

jeanluct commented 9 years ago

Ok, see what you think. You need to delete eqfuzzy.mex. Note that I changed the 3rd arg to being a delta instead of digits (more natural in this setting).

For now there is no user-specified flag. We first need to make sure it all works ok, and also fix #116.

jeanluct commented 9 years ago

See commit 65cf259 (typo in issue number), as well as b89b285, 02783b2.

jeanluct commented 9 years ago

Closing this issue, as it only relates to the non-MEX part and seems resolved.