Opened 4 years ago
Closed 4 years ago
#23164 closed enhancement (fixed)
cache Polynomial.is_irreducible()
Reported by: | saraedum | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-8.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()