-
Notifications
You must be signed in to change notification settings - Fork 5
Fix PD multiple components bug #2
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
base: master
Are you sure you want to change the base?
Conversation
|
Is this PR about #5? |
|
I'm sorry, but I didn't look into this for over 2 years, so actually I don't know anymore. But if I remember correctly, the problem I encountered had something to do with the braid on two strands given for example by \sigma \circ \sigma^{-1}. Because in that case there are two components that have only half the size of twice the length of the PD notation. |
Thanks for your reply! In the meantime I used the (inofficial) PyPI version to check if your changes fix #5. In order to do that I manipulated the code downloaded from PyPI locally with your changes. The result is that your changes actually fix the issue! Here is the Sage-code I used for the check: @LLewark what is standing against merging this PR? It fixes the wrong results of 1091 links from LinkInfo and the argumentation of @hexaron sounds reasonable to me. |
soehms
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above!
There is a bug, when a PD component loops back on itself.
In this case, the numbers
banddin the crossing[a, b, c, d]are not one apart.Therefore the
ifin:khoca/khoca.py
Lines 48 to 52 in dbb4a80
is not sufficient to catch this case for multiple components, since in this case
2 * len(pd)is not the size of the component.If we knew the size of the component, we could mod by that after shifting the indices to
0, but I think the simpler solution is to do as I suggest in this pull-request.