cayley bug resolved #40563
cayley bug resolved #40563
Conversation
|
please check and let me know if there are any issues <3 |
|
you may just use |
|
@cxzhong can you check it now? |
|
Can you test whether the bug has been fixed and add a new test in doctest |
|
In which situation may the exception be raised? Prefer to use proper capitalization and Markdown formatting. Prefer to use imperative mood for commit messages and pull request titles. |
|
I have added a doctest, and here's the same code working as expected, you can see that it shows 52 instead of 109 
|
Yes, I confirm this. @orlitzky Can you help me check it? |
| sage: hash(id1) == hash(id2) | ||
| True | ||
|
|
||
| TESTS:: |
There was a problem hiding this comment.
only one colon here. Make sure the docstring is valid RST.
|
What was the cause of the original bug, two equal elements with different hashes? If the |
You are right! I completely forgot about where the hash element was inherited from. I was debugging it in I just checked that The issue was, in a free group, elements are only equal if their words are identical, so the hash contract is naturally satisfied. But in a quotient (finitely presented) group, different words can represent the same element due to relations, so the inherited hash method is not sufficient. I'll remove the extra unecessary |
|
Can you check this? I test the bug is solved. @orlitzky |
src/sage/groups/free_group.py
Outdated
| self.parent()._confluent_rewriting_system = rs | ||
| canonical_form = self.parent()._confluent_rewriting_system.reduce(self) | ||
| return hash(str(canonical_form)) | ||
| except Exception: |
There was a problem hiding this comment.
What sort of exception are you expecting here? (Can you be more specific than Exception)? You probably don't want to ignore it if, say, the user presses Ctrl-C in the middle of the computation.
There was a problem hiding this comment.
ahh I should be specifying expected exceptions
got it!
ill add them now
|
can you explain that why there are four error types？ |
In |
|
do add a doctest to demonstrate each type of exception being thrown. Also, the current implementation, if the computation takes e.g. a second and then throw, trying to hash other elements will again try to recompute that. Not a good idea. the surrounding context is questionable also (specifically, currently I recommend moving the logic to |
|
should I directly work on the |
Description
This pull request resolves a bug in the Cayley computation logic. The issue caused incorrect results when handling certain edge cases in the Cayley transform. The fix updates the relevant function to correctly handle these cases, ensuring accurate mathematical results.
Addresses the root cause of the Cayley bug.
Adds tests to cover the previously failing scenarios.
Updates documentation to reflect the corrected behavior.
Fixes #40549.