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)

trac_13848.patch (84.9 KB) - added by malb 9 years ago.
rebased to 5.13.beta3

Download all attachments as: .zip

Change History (15)

comment:1 Changed 10 years ago by malb

Status: newneeds_review

comment:2 Changed 10 years ago by malb

Dependencies: 13847#13847

comment:3 Changed 10 years ago by AlexanderDreyer

The patch is fine. When bot green-lits, I can give a positive review.

comment:4 Changed 10 years ago by 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

comment:5 in reply to:  4 Changed 10 years ago by AlexanderDreyer

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 malb

I agree with AlexanderDreyer?, this is out of scope. Feel free to open a new ticket though.

comment:7 Changed 9 years ago by jdemeyer

Milestone: sage-5.11sage-5.12

comment:8 Changed 9 years ago by malb

Can we set this to positive review? It holds up #13849 and I just got another bug report that is due to #13849.

comment:9 Changed 9 years ago by AlexanderDreyer

Status: needs_reviewpositive_review

Hi Martin! Of course, my intention was to give a positive review then.

comment:10 Changed 9 years ago by malb

Thanks, Alexander!

comment:11 Changed 9 years ago by jdemeyer

Reviewers: Alexander Dreyer
Status: positive_reviewneeds_work

This needs to be rebased to sage-5.13.beta3.

Changed 9 years ago by malb

Attachment: trac_13848.patch added

rebased to 5.13.beta3

comment:12 Changed 9 years ago by malb

Status: needs_workneeds_review

rebased

comment:13 Changed 9 years ago by AlexanderDreyer

Status: needs_reviewpositive_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 jdemeyer

Merged in: sage-5.13.beta5
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.