-
-
Notifications
You must be signed in to change notification settings - Fork 553
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
Revision of the knot theory colorings method #36333
Conversation
Documentation preview for this PR (built with commit e3f62be; changes) is ready! 🎉 |
src/sage/knots/link.py
Outdated
@@ -3104,9 +3101,6 @@ def is_colorable(self, n): | |||
|
|||
- ``n`` -- the number of colors to consider |
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.
I would add the same comment as before:
- ``n`` -- the number of colors to consider (if ommitted the
value of the determinant of ``self`` will be taken
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.
Thanks! I've done it in the current commit.
src/sage/knots/link.py
Outdated
""" | ||
return self._coloring_matrix(n).nullity() > 1 | ||
try: |
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.
Is there a reason to try to use nullity
instead of going directly for the way that always works?
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.
My intention is to maintain performance on prime numbers. It would decrease by a factor of 28:
sage: KnotInfo.K12a_1007.inject()
Defining K12a_1007
sage: K12a_1007.determinant()
143
sage: K = K12a_1007.link()
sage: M = K._coloring_matrix(n=13)
sage: %timeit M.nullity()
1.47 µs ± 169 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)
sage: %timeit M.right_kernel_matrix().dimensions()[0]
41.5 µs ± 1.09 µs per loop (mean ± std. dev. of 7 runs, 10,000 loops each)
For just one call this is negligible. But if invoked inside a large loop this could count.
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.
In that case, maybe checking for the ring to be a field would be better than trying and catching the exception.
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.
In that case, maybe checking for the ring to be a field would be better than trying and catching the exception.
I agree! I just made the change.
Automated tests were cancelled for some reason. Did you test it all? |
Only below |
If a full check is successful, it is ok from my part. |
I also had problems with doctests on my fastest machine (an import error on In the meantime there is a successful bot test result triggered by synchronizing with |
The following issue has been reported by Chuck Livingston to me:
Since 13 divides the determinant of the knot 156 colorings are expected. While analyzing this issue, I found that the implementation (done in #21863) does not match the definition of Fox
n
-colorings given in the listed references on Wikipedia and in Chuck's book.The name coloring maybe misleading here: The number
n
does not refer to the number of (arbitrary) colors to be used. It can be interpreted as the size of a fixed color palette. Mathematically, the colors correspond to elements of then
-th dihedral groupD_n
whose rotations (multiplied with the generating transvection) form the color palette. A coloring corresponds to a group homomorphism from the fundamental groupF
of the knot toD_n
. If one interprets the arcs of the knot as the generators ofF
, their colors correspond to their images inD_n
.Here are some lines in the current code that do not meet this definition:
n
to the next prime number:n
to the number of different colors used:This PR fixes these issues. Furthermore, a new method
coloring_maps
is added to calculate the group homomorphisms explicitly. This can also be used to verify the results of the methodcolorings
. Finally, theplot
method is adapted to the changes.Note that the definition does not restrict
n
to be a prime number. However, if this is not the case, we cannot just take the next prime number to work more comfortably over a field. It is still necessary to work over the residue class ring modulon
. Although the methodright_kernel_matrix
works well in this context the methodright_kernel
does not work due to a failure in thespan
method of theFreeModule
class. Since I didn't find an easy fix for this I worked around this issue.📝 Checklist
⌛ Dependencies