Opened 4 years ago
Closed 4 years ago
#23164 closed enhancement (fixed)
cache Polynomial.is_irreducible()
Reported by:  saraedum  Owned by:  

Priority:  minor  Milestone:  sage8.0 
Component:  commutative algebra  Keywords:  sd86.5 
Cc:  Merged in:  
Authors:  Julian Rüth, Aly Deines  Reviewers:  David Roe 
Report Upstream:  N/A  Work issues:  doctest that something is in the cache 
Branch:  9d9e878 (Commits)  Commit:  9d9e878f058ce3f331c20165e62c4fd4c212692f 
Dependencies:  Stopgaps: 
Description
Determining irreducibility is often very expensive. At the same time, it requires hardly any memory to cache this.
Change History (19)
comment:1 Changed 4 years ago by
 Branch set to u/saraedum/cache_polynomial_is_irreducible__
comment:2 Changed 4 years ago by
 Commit set to 78c95871a5fd9fce1f49ce06dc5b6cf17573e36c
comment:3 Changed 4 years ago by
 Status changed from new to needs_review
I took the liberty to polish the docstrings a bit while I was at it. Some comments:
 I removed the mention of composite n from the generic docstring since this is not the documentation that you see when you look at the docstring for a polynomial over
Z/nZ
.  I removed the mention of ticket 5140 since I found the discussion there confusing and replaced it with the conclusion of that discussion.
 I did not add a doctest for the actual caching. How would I do that (without checking
type(f.is_irreducible)
?)
comment:4 Changed 4 years ago by
 Status changed from needs_review to needs_work
 Work issues set to doctest that something is in the cache
comment:5 Changed 4 years ago by
Actually, this isn't caching:
sage: S.<s> = Zmod(7)[] sage: s.is_irreducible() True sage: s.is_irreducible.cache sage: s.is_irreducible.cache is None True
comment:6 Changed 4 years ago by
 Branch changed from u/saraedum/cache_polynomial_is_irreducible__ to u/aly.deines/cache_polynomial_is_irreducible__
comment:7 Changed 4 years ago by
 Commit changed from 78c95871a5fd9fce1f49ce06dc5b6cf17573e36c to 9c2a4e35580c113856c371b6c1476664f45d1547
 Status changed from needs_work to needs_review
This should be caching now. I also added doctests.
New commits:
9c2a4e3  Added line to polynomial_element.pyx to allow caching and added doctests for cached methods.

comment:8 Changed 4 years ago by
comment:9 Changed 4 years ago by
 Branch changed from u/aly.deines/cache_polynomial_is_irreducible__ to u/roed/cache_polynomial_is_irreducible__
comment:10 Changed 4 years ago by
 Commit changed from 9c2a4e35580c113856c371b6c1476664f45d1547 to 9d9e878f058ce3f331c20165e62c4fd4c212692f
 Status changed from needs_review to positive_review
I fixed a doctest error, and Julian okayed it verbally. I ran tests locally, so giving this a positive review.
New commits:
9d9e878  Fix doctest error in sage/structure/misc.pyx

comment:11 Changed 4 years ago by
 Reviewers set to David Roe
comment:12 Changed 4 years ago by
 Branch changed from u/roed/cache_polynomial_is_irreducible__ to u/saraedum/cache_polynomial_is_irreducible__
comment:13 Changed 4 years ago by
 Commit changed from 9d9e878f058ce3f331c20165e62c4fd4c212692f to 2cf1581cc9393cab2eb751915d0ecd1815cd4c74
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
2cf1581  cache Polynomial.is_squarefree()

comment:14 Changed 4 years ago by
 Commit changed from 2cf1581cc9393cab2eb751915d0ecd1815cd4c74 to 78c95871a5fd9fce1f49ce06dc5b6cf17573e36c
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
comment:15 Changed 4 years ago by
 Status changed from needs_review to positive_review
pushed to the wrong ticket number again, sorry.
comment:16 Changed 4 years ago by
 Commit changed from 78c95871a5fd9fce1f49ce06dc5b6cf17573e36c to 2cf1581cc9393cab2eb751915d0ecd1815cd4c74
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
2cf1581  cache Polynomial.is_squarefree()

comment:17 Changed 4 years ago by
 Branch changed from u/saraedum/cache_polynomial_is_irreducible__ to u/roed/cache_polynomial_is_irreducible__
 Commit changed from 2cf1581cc9393cab2eb751915d0ecd1815cd4c74 to 9d9e878f058ce3f331c20165e62c4fd4c212692f
comment:18 Changed 4 years ago by
 Status changed from needs_review to positive_review
ok. I think I reverted to the positive_review state now.
comment:19 Changed 4 years ago by
 Branch changed from u/roed/cache_polynomial_is_irreducible__ to 9d9e878f058ce3f331c20165e62c4fd4c212692f
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
cache polynomial_element.is_irreducible()