Opened 10 years ago
Closed 9 years ago
#13848 closed enhancement (fixed)
mq.SR: use deglex and polybori (if GF(2)) by default
| Reported by: | malb | Owned by: | mvngu |
|---|---|---|---|
| Priority: | major | Milestone: | sage-5.13 |
| Component: | cryptography | Keywords: | |
| Cc: | PolyBoRi, AlexanderDreyer, cosh | Merged in: | sage-5.13.beta5 |
| Authors: | Martin Albrecht | Reviewers: | Alexander Dreyer |
| Report Upstream: | N/A | Work issues: | |
| Branch: | Commit: | ||
| Dependencies: | #13847 | Stopgaps: |
Status badges
Description
It is silly that the default for equation systems over GF(2) as produced by mq.SR is !Singular and not PolyBoRi.
The attached patch fixes that and also changes the default term ordering to "deglex" which is a PolyBoRi native ordering ("degrevlex" is not).
This patch depends on #13847 which removes some deprecated code.
Attachments (1)
Change History (15)
comment:1 Changed 10 years ago by
| Status: | new → needs_review |
|---|
comment:2 Changed 10 years ago by
| Dependencies: | 13847 → #13847 |
|---|
comment:3 Changed 10 years ago by
comment:4 follow-up: 5 Changed 10 years ago by
Maybe one can also use this patch to correct the following things (found by pyflakes):
sage/crypto/mq/sr.py:2453: local variable 'n' is assigned to but never used sage/crypto/mq/sr.py:3180: local variable 'n' is assigned to but never used
comment:5 Changed 10 years ago by
Replying to chapoton:
Maybe one can also use this patch to correct the following things (found by pyflakes):
sage/crypto/mq/sr.py:2453: local variable 'n' is assigned to but never used sage/crypto/mq/sr.py:3180: local variable 'n' is assigned to but never used
I wouldn't do so, because it's a completely unrelated problem (as far as I understand). So for documentation reasons this should be a new ticket.
BTW: the fix itself is trivial: one just has to remove the corresponding lines of the form n = self._n.
comment:6 Changed 10 years ago by
I agree with AlexanderDreyer?, this is out of scope. Feel free to open a new ticket though.
comment:7 Changed 9 years ago by
| Milestone: | sage-5.11 → sage-5.12 |
|---|
comment:8 Changed 9 years ago by
comment:9 Changed 9 years ago by
| Status: | needs_review → positive_review |
|---|
Hi Martin! Of course, my intention was to give a positive review then.
comment:11 Changed 9 years ago by
| Reviewers: | → Alexander Dreyer |
|---|---|
| Status: | positive_review → needs_work |
This needs to be rebased to sage-5.13.beta3.
comment:13 Changed 9 years ago by
| Status: | needs_review → positive_review |
|---|
I, browsed the new patch. it looks find. Assuming that the patch will apply, I'll re-install the positive review.
comment:14 Changed 9 years ago by
| Merged in: | → sage-5.13.beta5 |
|---|---|
| Resolution: | → fixed |
| Status: | positive_review → closed |
The patch is fine. When bot green-lits, I can give a positive review.