Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problems with colorbraiding and taffy (MEX) #116

Closed
jeanluct opened this issue Jan 9, 2015 · 7 comments
Closed

Problems with colorbraiding and taffy (MEX) #116

jeanluct opened this issue Jan 9, 2015 · 7 comments
Assignees
Labels
Milestone

Comments

@jeanluct
Copy link
Owner

jeanluct commented Jan 9, 2015

There is a discrepancy between the guide and current results. In fact there are quite a few.

  • With BRAIDLAB_braid_nomex=0, taffy('4rods',pi/2) returns a different error message:
Error using cross2gen_helper
Attempting to apply crossings resulted in inconsistency. Concurrent block at time
100.5, 4 crossings between 1 and 5 cannot be resolved.

This is a bit cryptic. Nothing about the projection line, unlike the old error message (see guide).

  • Finally, still using the MEX files,
>> taffy('4rods')
Error using cross2gen_helper
Attempting to apply crossings resulted in inconsistency. Concurrent block at time
50.75, 2 crossings between 0 and 2 cannot be resolved.

still gives the error message, even though there should now be a well-defined braid.

So it sounds like we've introduced a regression of some kind. This is important and must be fixed before 3.1.

We should also see if this was ok as far back as 3.0.1.

(Split the non-MEX part of this to #117.)

@jeanluct
Copy link
Owner Author

jeanluct commented Jan 9, 2015

See 3909cf0. You can now set a property prop('BraidAbsTol',1e-10). This is queried in colorbraiding and passed to cross2gen.m. You just need to do the same for the C++ code and rewrite the trivial eqfuzzy. Unless you want to keep your old version of eqfuzzy around, but if we need it later we could always go back to it.

I was going to have eqfuzzy query that property, but that would make it slow, and also make it difficult to implement in C++.

@mbudisic
Copy link
Collaborator

mbudisic commented Jan 9, 2015

No need for that. In fact, we should probably get rid of eqfuzzy if we are going to stick with absolute tolerance as it makes the code less readable for such a simple operation.

In any case, there is a new problem right now that fails on taffy but I don't have time to fix it before heading out.

@jeanluct
Copy link
Owner Author

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

@jeanluct
Copy link
Owner Author

I don't think it's a new problem: it's the second one mentioned at the start of this issue. I think there were two problems, one with eqfuzzy but the other seems to be that the C++ code doesn't like simultaneous crossings somehow. You can try to bisect but I'm pretty sure that the parallel version of taffy('4rods') has never actually worked.

jeanluct added a commit that referenced this issue Jan 10, 2015
mbudisic added a commit that referenced this issue Jan 10, 2015
Bug was in incorrect reporting of whether all crossings were applied,
success was accidentally reported by "false" flag instead of "true"
in applyCrossings() function.
@mbudisic mbudisic reopened this Jan 11, 2015
@mbudisic
Copy link
Collaborator

taffyTest fails sometimes, literally sometimes. Out of 5 repeated runs of run(taffyTest) , 4 times it passed, 1 time it failed because these two braids are not lexically equal, even though they are equal to each other:

    Actual Object:
         < 2  1  2  5  4  5  3  3  2  1  2  4  5  4 >
    Expected Object:
         < 2  1  2  4  5  4  3  3  2  1  2  4  5  4 >

I am guessing the issue is that a concurrent pair of crossings C1, C2 can be resolved equally as "C1 - C2" braid, and "C2 - C1" braid. I think this has to do with multithreaded execution, as different pairs of strands may be processed on different cores. If this happens, then depending on what else is going on at the computer, one core might terminate a bit ahead, or after, the other, which would affect the order in which their results are processed by the part of the algorithm that "integrates" pairwise crossings into a braid.

I have not managed to get the test to fail in the manner above when BRAIDLAB_threads = 1, but I did manage to do it in multithreaded mode.
(I accidentally filed this under #117 instead...)

@mbudisic mbudisic changed the title Problems with colorbraiding and taffy Problems with colorbraiding and taffy (MEX) Jan 11, 2015
@jeanluct
Copy link
Owner Author

What is ABSTOL_TIME? There's no fragility involved in having this parameter?

@jeanluct
Copy link
Owner Author

In any case, I changed the unit test to check for braid equality, and now it works fine. It would be nice if we could guarantee that the answer returned was always identical, but being the correct braid is more important.

I will close this and go to 3.1, but also open an issue (#118).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants