Refactor power method to handle negative exponents #159
Conversation
The recursive implementation of __pow__ reaches the maximum recursion depth on s390x during pairing tests. This patch converts it to an iterative square-and-multiply algorithm.
|
The error on s390x was previously visible in Debian that builds this package for s390x. This patch for applied in https://salsa.debian.org/python-team/packages/python-py-ecc/-/commit/5b1df54f1a9079e9b70a06219bad59fc23e6417a and now the s390x tests pass on Debian, proving this patch is correct: 
|
I'm quite confused, this patch does not represent the code in this PR. It's closer to the code in lines 148-156 here. Is this what the PR means to patch? Can the problem be explained a bit more and can we get a consensus on which code needs to be patched? The code in the PR changes does not seem to have this issue. I'd also like to know which tests are failing. If they are pairing tests in this repo, are we able to reproduce this with different parametrized numbers on other platforms? And can we then show that it fails without the patch and the test passes with the patch? Thanks. |
|
Yes it is the same patch in this PR and what was included in the Debian package debian/patches/fix-recursion-limit-s390x.patch, as seen added in the commit linked above. The screenshot above is from https://ci.debian.net/packages/p/python-py-ecc/unstable/s390x/ showing version 8.0.0-1 was failing, and now the latest version in Debian that shipped with this patch 8.0.0-2 is passing. |
The diff you are linking to is my point. This is not the code that is being changed here. The diff in your link here, does not match the diff in the PR here. This is not the same code. Please explain the issue better. If there is an issue with the removed code in this patch, please rephrase your problem and point to a debian patch that has a similar code change. Please add a relevant test that highlights the issue in this PR. |
|
Otto, I apologize. It seems fselmo is right... I made a mistake processing the code for this PR. However, fselmo, you should take into account the information Otto is giving you about how a situation in the s390x architecture was corrected in the Debian packaging. Give me some time to correct the PR. |
3 participants
The recursive implementation of pow reaches the maximum recursion depth on s390x during pairing tests. This patch converts it to an iterative square-and-multiply algorithm.