Opened 9 years ago
Closed 5 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, GitHub, GitLab)  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 9 years ago by
Branch:  → u/saraedum/ticket/16339 

Created:  May 12, 2014, 8:43:44 PM → May 12, 2014, 8:43:44 PM 
Modified:  May 12, 2014, 8:43:44 PM → May 12, 2014, 8:43:44 PM 
comment:2 Changed 9 years ago by
Status:  new → needs_review 

comment:3 Changed 9 years ago by
Commit:  → fddc853b27aaacd89d18ac1958742b343bb97ae7 

Status:  needs_review → needs_work 
comment:4 Changed 9 years ago by
Milestone:  sage6.3 → sage6.4 

comment:5 Changed 7 years ago by
Commit:  fddc853b27aaacd89d18ac1958742b343bb97ae7 → 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 7 years ago by
Dependencies:  #16321 → #15692 

comment:7 Changed 7 years ago by
Commit:  07a1e2e464b4273c531ec6cccee89a42d13d8af1 → 0a788023afb9492f872f1da5cd44ce8a74c24f09 

comment:8 Changed 7 years ago by
Status:  needs_work → needs_review 

comment:9 Changed 7 years ago by
Keywords:  days71 added 

comment:10 Changed 7 years ago by
Branch:  u/saraedum/ticket/16339 → u/roed/ticket/16339 

comment:11 Changed 7 years ago by
Commit:  0a788023afb9492f872f1da5cd44ce8a74c24f09 → 37499a44d839bd61a1920636fbbc68146b65c2f8 

Reviewers:  → David Roe 
Status:  needs_review → 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:13 Changed 6 years ago by
Branch:  u/roed/ticket/16339 → u/aly.deines/ticket/16339 

Commit:  37499a44d839bd61a1920636fbbc68146b65c2f8 → 12e9c4379d64eec9e283ad1be3a356abd6afa8a6 
comment:14 Changed 6 years ago by
Commit:  12e9c4379d64eec9e283ad1be3a356abd6afa8a6 → c803a5f02d9925f98d603ddf8e72cb4d7496e8ec 

Branch pushed to git repo; I updated commit sha1. New commits:
c803a5f  Fixed some imports.

comment:15 Changed 6 years ago by
Status:  needs_work → needs_review 

comment:16 Changed 5 years ago by
Status:  needs_review → needs_work 

Merge conflict with 8.1.beta3 needs to be resolved.
comment:17 Changed 5 years ago by
Branch:  u/aly.deines/ticket/16339 → u/saraedum/ticket/16339 

comment:18 Changed 5 years ago by
Branch:  u/saraedum/ticket/16339 → u/aly.deines/ticket/16339 

Status:  needs_work → 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 5 years ago by
Branch:  u/aly.deines/ticket/16339 → u/saraedum/ticket/16339 

comment:20 Changed 5 years ago by
Commit:  c803a5f02d9925f98d603ddf8e72cb4d7496e8ec → 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 5 years ago by
Status:  needs_review → positive_review 

comment:22 Changed 5 years ago by
Branch:  u/saraedum/ticket/16339 → 710ab456b3ec97e186ec5800fb3b5aac90e159d4 

Resolution:  → fixed 
Status:  positive_review → 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.