Opened 6 years ago
Closed 3 years ago
#16339 closed enhancement (fixed)
Improve caching for HeckeAlgebra
Reported by:  saraedum  Owned by:  

Priority:  trivial  Milestone:  sage6.4 
Component:  modular forms  Keywords:  days71 
Cc:  Merged in:  
Authors:  Julian Rüth  Reviewers:  David Roe 
Report Upstream:  N/A  Work issues:  
Branch:  710ab45 (Commits)  Commit:  710ab456b3ec97e186ec5800fb3b5aac90e159d4 
Dependencies:  #15692  Stopgaps: 
Description
Hecke algebras use adhoc caching which causes trouble for #11895. This ticket replaces some of the caching with CachedRepresentation
and cached_method
.
Change History (22)
comment:1 Changed 6 years ago by
 Branch set to u/saraedum/ticket/16339
 Created changed from 05/12/14 20:43:44 to 05/12/14 20:43:44
 Modified changed from 05/12/14 20:43:44 to 05/12/14 20:43:44
comment:2 Changed 6 years ago by
 Status changed from new to needs_review
comment:3 Changed 6 years ago by
 Commit set to fddc853b27aaacd89d18ac1958742b343bb97ae7
 Status changed from needs_review to needs_work
comment:4 Changed 6 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:5 Changed 4 years ago by
 Commit changed from fddc853b27aaacd89d18ac1958742b343bb97ae7 to 07a1e2e464b4273c531ec6cccee89a42d13d8af1
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
c182f40  Remove ClearCacheOnPickle meta class

e3a7eb4  Do not pickle cached methods

07af453  Fix incorrect usage of key parameter of @cached_method for ambient spaces of modular forms

31f5e5b  HeckeAlgebra and HeckeAlgebra_anemic inherit from CachedRepresentation

7174a49  HeckeAlgebra uses @cached_method where possible

07a1e2e  Fixed the expected output of an unstable eigenvalue test.

comment:6 Changed 4 years ago by
 Dependencies changed from #16321 to #15692
comment:7 Changed 4 years ago by
 Commit changed from 07a1e2e464b4273c531ec6cccee89a42d13d8af1 to 0a788023afb9492f872f1da5cd44ce8a74c24f09
comment:8 Changed 4 years ago by
 Status changed from needs_work to needs_review
comment:9 Changed 4 years ago by
 Keywords days71 added
comment:10 Changed 4 years ago by
 Branch changed from u/saraedum/ticket/16339 to u/roed/ticket/16339
comment:11 Changed 4 years ago by
 Commit changed from 0a788023afb9492f872f1da5cd44ce8a74c24f09 to 37499a44d839bd61a1920636fbbc68146b65c2f8
 Reviewers set to David Roe
 Status changed from needs_review to positive_review
I fixed a few problems, with Julian looking over my shoulder.
New commits:
37499a4  Actually removed some caches that were partially removed from ambient modular forms spaces

comment:12 Changed 4 years ago by
 Status changed from positive_review to needs_work
Merge conflict with #12603.
comment:13 Changed 4 years ago by
 Branch changed from u/roed/ticket/16339 to u/aly.deines/ticket/16339
 Commit changed from 37499a44d839bd61a1920636fbbc68146b65c2f8 to 12e9c4379d64eec9e283ad1be3a356abd6afa8a6
comment:14 Changed 4 years ago by
 Commit changed from 12e9c4379d64eec9e283ad1be3a356abd6afa8a6 to c803a5f02d9925f98d603ddf8e72cb4d7496e8ec
Branch pushed to git repo; I updated commit sha1. New commits:
c803a5f  Fixed some imports.

comment:15 Changed 4 years ago by
 Status changed from needs_work to needs_review
comment:16 Changed 3 years ago by
 Status changed from needs_review to needs_work
Merge conflict with 8.1.beta3 needs to be resolved.
comment:17 Changed 3 years ago by
 Branch changed from u/aly.deines/ticket/16339 to u/saraedum/ticket/16339
comment:18 Changed 3 years ago by
 Branch changed from u/saraedum/ticket/16339 to u/aly.deines/ticket/16339
 Status changed from needs_work to needs_review
Since this had positive review only merge commits were added. I guess I can set this back to positive review once the patchbot is happy.
comment:19 Changed 3 years ago by
 Branch changed from u/aly.deines/ticket/16339 to u/saraedum/ticket/16339
comment:20 Changed 3 years ago by
 Commit changed from c803a5f02d9925f98d603ddf8e72cb4d7496e8ec to 710ab456b3ec97e186ec5800fb3b5aac90e159d4
I would say yes unless the merge conflicts were nontrivial.
New commits:
710ab45  Merge branch 'develop' into t/16339/ticket/16339

comment:21 Changed 3 years ago by
 Status changed from needs_review to positive_review
comment:22 Changed 3 years ago by
 Branch changed from u/saraedum/ticket/16339 to 710ab456b3ec97e186ec5800fb3b5aac90e159d4
 Resolution set to fixed
 Status changed from positive_review to closed
Doctest failure:
(The patchbot's
git
was able to merge the branch, unlike the Tracgit
plugin.)Last 10 new commits:
Removed sage.misc.cachefunc.ClearCacheOnPickle
Propagate key of a @cached_method correctly
Merge branch 'u/saraedum/ticket/16337' of git://trac.sagemath.org/sage into ticket/15692
Added a pickle parameter for @cached_method
Enable pickling of the cache for groebner_basis()
Merge branch 'u/saraedum/ticket/15692' of git://trac.sagemath.org/sage into ticket/16321
Fix incorrect usage of key parameter of @cached_method for ambient spaces of modular forms
HeckeAlgebra and HeckeAlgebra_anemic inherit from CachedRepresentation
HeckeAlgebra uses @cached_method where possible
Fixed the expected output of an unstable eigenvalue test.