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 saraedum

  • Branch set to u/saraedum/cache_polynomial_is_irreducible__

comment:2 Changed 4 years ago by git

  • Commit set to 78c95871a5fd9fce1f49ce06dc5b6cf17573e36c

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​78c9587cache polynomial_element.is_irreducible()

comment:3 Changed 4 years ago by saraedum

  • 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 saraedum

  • 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 aly.deines

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 aly.deines

  • Branch changed from u/saraedum/cache_polynomial_is_irreducible__ to u/aly.deines/cache_polynomial_is_irreducible__

comment:7 Changed 4 years ago by aly.deines

  • 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:

​9c2a4e3Added line to polynomial_element.pyx to allow caching and added doctests for cached methods.

comment:8 Changed 4 years ago by aly.deines

  • Authors changed from Julian Rüth to Julian Rüth, Aly Deines

comment:9 Changed 4 years ago by roed

  • Branch changed from u/aly.deines/cache_polynomial_is_irreducible__ to u/roed/cache_polynomial_is_irreducible__

comment:10 Changed 4 years ago by roed

  • 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:

​9d9e878Fix doctest error in sage/structure/misc.pyx

comment:11 Changed 4 years ago by roed

  • Reviewers set to David Roe

comment:12 Changed 4 years ago by saraedum

  • Branch changed from u/roed/cache_polynomial_is_irreducible__ to u/saraedum/cache_polynomial_is_irreducible__

comment:13 Changed 4 years ago by git

  • 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:

​2cf1581cache Polynomial.is_squarefree()

comment:14 Changed 4 years ago by git

  • 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 saraedum

  • Status changed from needs_review to positive_review

pushed to the wrong ticket number again, sorry.

comment:16 Changed 4 years ago by git

  • 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:

​2cf1581cache Polynomial.is_squarefree()

comment:17 Changed 4 years ago by saraedum

  • 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 saraedum

  • 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 vbraun

  • Branch changed from u/roed/cache_polynomial_is_irreducible__ to 9d9e878f058ce3f331c20165e62c4fd4c212692f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.