Opened 10 years ago
Closed 10 years ago
#1541 closed enhancement (fixed)
[with patch, postive review] improve PolyBoRi integration
Reported by: | malb | Owned by: | malb |
---|---|---|---|
Priority: | major | Milestone: | sage-2.10 |
Component: | commutative algebra | Keywords: | |
Cc: | burcin | Merged in: | |
Authors: | Reviewers: | ||
Report Upstream: | Work issues: | ||
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
The attached patch provides:
mq.SR
can now construct PolyBoRi? polynomial systems- some comments added to
pbori.pyx
BooleanPolynomial.__hash__
BooleanPolynomial.variables
- coercion of GF(2) elements to
BooleanPolynomialRing
BooleanPolynomialRing.__call__
accepts strings_sig_on
/_sig_off
aroundgroebner_basis
call
Attachments (1)
Change History (12)
comment:1 Changed 10 years ago by
- Summary changed from [with patch] improve PolyBoRi integration to [with patch, negative review] improve PolyBoRi integration
comment:2 Changed 10 years ago by
results of
./sage -t -valgrind devel/sage-main/sage/crypto/mq/sr.py
are at
http://sage.math.washington.edu/home/rlmill/.sage/sage-memcheck.850
comment:3 Changed 10 years ago by
Disregard those valgrind results- they are useless.
comment:4 Changed 10 years ago by
- Summary changed from [with patch, negative review] improve PolyBoRi integration to [with patch] improve PolyBoRi integration
I think the negative review has been resolved, and the currRing patch attached to this ticket should be applied (given a positive review) soon.
comment:5 Changed 10 years ago by
Following Martin's comments on the slowdown caused by using __getattr__
in BooleanPolynomial
, attached patch with file name polybori_booleanpolynomial_getattr.patch
makes the methods directly available.
Some timings (using random polynomials in each case might not be a good idea, but it still demonstrates the point):
Without the patch:
sage: P = BooleanPolynomialRing(100,'x') sage: from polybori.randompoly import gen_random_poly sage: p = gen_random_poly(int(100)) sage: %timeit s = p.set() 100000 loops, best of 3: 2.85 µs per loop sage: %timeit d = p.deg() 100000 loops, best of 3: 2.26 µs per loop sage: %timeit m = p.lead() 100000 loops, best of 3: 6.7 µs per loop sage: %timeit n = p.navigation() 100000 loops, best of 3: 2.82 µs per loop sage: %timeit c = p.constant() 100000 loops, best of 3: 2.02 µs per loop
With the patch:
sage: %timeit s = p.set() 1000000 loops, best of 3: 540 ns per loop sage: %timeit d = p.deg() 1000000 loops, best of 3: 382 ns per loop sage: %timeit m = p.lead() 100000 loops, best of 3: 3.76 µs per loop sage: %timeit n = p.navigation() 1000000 loops, best of 3: 453 ns per loop sage: %timeit c = p.constant() 1000000 loops, best of 3: 305 ns per loop
comment:6 follow-ups: ↓ 7 ↓ 8 Changed 10 years ago by
attachment:polybori-coercion.patch provides coercion enhancements and small fixes to pbori.pyx
. It should be applied after attachment:polybori_booleanpolynomial_getattr.patch.
This patch adds coercion from ZZ
, and GF(2)
to BooleanPolynomialRing
, so Martin's patch needs to be revised.
comment:7 in reply to: ↑ 6 Changed 10 years ago by
- Cc burcin added
attachment:polybori-coercion.patch is now ticket #1667.
comment:8 in reply to: ↑ 6 Changed 10 years ago by
Replying to burcin:
This patch adds coercion from
ZZ
, andGF(2)
toBooleanPolynomialRing
, so Martin's patch needs to be revised.
In the sense that the comments are outdated?
comment:9 Changed 10 years ago by
The currRing.patch
is now #1701 because it doesn't logically belong here.
comment:10 Changed 10 years ago by
- Summary changed from [with patch] improve PolyBoRi integration to [with patch, postive review] improve PolyBoRi integration
Deleted original patch, just use burcin's getattr patch.
comment:11 Changed 10 years ago by
- Resolution set to fixed
- Status changed from new to closed
Merged in Sage 2.10.alpha0.
Caution: when patching the offset was very large: Hunk #1 succeeded at 1372 (offset 284 lines).
I did verify that indeed the section was moved that far down in the current sources.
Cheers,
Michael
On sage.math, applied to alpha2: