Opened 9 years ago

Closed 5 years ago

#16339 closed enhancement (fixed)

Improve caching for HeckeAlgebra

Reported by: saraedum Owned by:
Priority: trivial Milestone: sage-6.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:

GitHub link to the corresponding issue

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 saraedum

Branch: u/saraedum/ticket/16339
Created: May 12, 2014, 8:43:44 PMMay 12, 2014, 8:43:44 PM
Modified: May 12, 2014, 8:43:44 PMMay 12, 2014, 8:43:44 PM

comment:2 Changed 9 years ago by saraedum

Status: newneeds_review

comment:3 Changed 9 years ago by pbruin

Commit: fddc853b27aaacd89d18ac1958742b343bb97ae7
Status: needs_reviewneeds_work

Doctest failure:

sage -t --long src/sage/modular/hecke/module.py
**********************************************************************
File "src/sage/modular/hecke/module.py", line 1646, in sage.modular.hecke.module.HeckeModule_free_module.system_of_eigenvalues
Failed example:
    [A.system_of_eigenvalues(3) for A in S.decomposition()]
Expected:
    [[1, 1, 0], [1, -1, -alpha - 1]]
Got:
    [[1, 1, 0], [1, -1, 1/2*alpha + 1/2]]
**********************************************************************
1 item had failures:
   1 of  14 in sage.modular.hecke.module.HeckeModule_free_module.system_of_eigenvalues
    [188 tests, 1 failure, 7.24 s]

(The patchbot's git was able to merge the branch, unlike the Trac git plugin.)


Last 10 new commits:

ce27b84Removed sage.misc.cachefunc.ClearCacheOnPickle
e27e316Propagate key of a @cached_method correctly
94fcddbMerge branch 'u/saraedum/ticket/16337' of git://trac.sagemath.org/sage into ticket/15692
72fce8bAdded a pickle parameter for @cached_method
0717849Enable pickling of the cache for groebner_basis()
561072fMerge branch 'u/saraedum/ticket/15692' of git://trac.sagemath.org/sage into ticket/16321
c74f601Fix incorrect usage of key parameter of @cached_method for ambient spaces of modular forms
034ea7aHeckeAlgebra and HeckeAlgebra_anemic inherit from CachedRepresentation
fe5ad0aHeckeAlgebra uses @cached_method where possible
fddc853Fixed the expected output of an unstable eigenvalue test.

comment:4 Changed 9 years ago by vbraun_spam

Milestone: sage-6.3sage-6.4

comment:5 Changed 7 years ago by git

Commit: fddc853b27aaacd89d18ac1958742b343bb97ae707a1e2e464b4273c531ec6cccee89a42d13d8af1

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

c182f40Remove ClearCacheOnPickle meta class
e3a7eb4Do not pickle cached methods
07af453Fix incorrect usage of key parameter of @cached_method for ambient spaces of modular forms
31f5e5bHeckeAlgebra and HeckeAlgebra_anemic inherit from CachedRepresentation
7174a49HeckeAlgebra uses @cached_method where possible
07a1e2eFixed the expected output of an unstable eigenvalue test.

comment:6 Changed 7 years ago by saraedum

Dependencies: #16321#15692
Last edited 7 years ago by saraedum (previous) (diff)

comment:7 Changed 7 years ago by git

Commit: 07a1e2e464b4273c531ec6cccee89a42d13d8af10a788023afb9492f872f1da5cd44ce8a74c24f09

Branch pushed to git repo; I updated commit sha1. New commits:

ec0e799fix imports in modular forms
0a78802fix cached_method key computation in modform.ambient_R

comment:8 Changed 7 years ago by saraedum

Status: needs_workneeds_review

New commits:

ec0e799fix imports in modular forms
0a78802fix cached_method key computation in modform.ambient_R

comment:9 Changed 7 years ago by saraedum

Keywords: days71 added

comment:10 Changed 7 years ago by roed

Branch: u/saraedum/ticket/16339u/roed/ticket/16339

comment:11 Changed 7 years ago by roed

Commit: 0a788023afb9492f872f1da5cd44ce8a74c24f0937499a44d839bd61a1920636fbbc68146b65c2f8
Reviewers: David Roe
Status: needs_reviewpositive_review

I fixed a few problems, with Julian looking over my shoulder.


New commits:

37499a4Actually removed some caches that were partially removed from ambient modular forms spaces

comment:12 Changed 7 years ago by pbruin

Status: positive_reviewneeds_work

Merge conflict with #12603.

comment:13 Changed 6 years ago by aly.deines

Branch: u/roed/ticket/16339u/aly.deines/ticket/16339
Commit: 37499a44d839bd61a1920636fbbc68146b65c2f812e9c4379d64eec9e283ad1be3a356abd6afa8a6

Fixed merge conflict.


New commits:

12e9c43Merged 16339 to develop.

comment:14 Changed 6 years ago by git

Commit: 12e9c4379d64eec9e283ad1be3a356abd6afa8a6c803a5f02d9925f98d603ddf8e72cb4d7496e8ec

Branch pushed to git repo; I updated commit sha1. New commits:

c803a5fFixed some imports.

comment:15 Changed 6 years ago by aly.deines

Status: needs_workneeds_review

comment:16 Changed 5 years ago by mderickx

Status: needs_reviewneeds_work

Merge conflict with 8.1.beta3 needs to be resolved.

comment:17 Changed 5 years ago by saraedum

Branch: u/aly.deines/ticket/16339u/saraedum/ticket/16339

comment:18 Changed 5 years ago by saraedum

Branch: u/saraedum/ticket/16339u/aly.deines/ticket/16339
Status: needs_workneeds_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 saraedum

Branch: u/aly.deines/ticket/16339u/saraedum/ticket/16339

comment:20 Changed 5 years ago by mderickx

Commit: c803a5f02d9925f98d603ddf8e72cb4d7496e8ec710ab456b3ec97e186ec5800fb3b5aac90e159d4

I would say yes unless the merge conflicts were nontrivial.


New commits:

710ab45Merge branch 'develop' into t/16339/ticket/16339

comment:21 Changed 5 years ago by mderickx

Status: needs_reviewpositive_review

comment:22 Changed 5 years ago by vbraun

Branch: u/saraedum/ticket/16339710ab456b3ec97e186ec5800fb3b5aac90e159d4
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.