Opened 6 years ago

Closed 3 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) 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 saraedum

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

  • Status changed from new to needs_review

comment:3 Changed 6 years ago by pbruin

  • Commit set to fddc853b27aaacd89d18ac1958742b343bb97ae7
  • Status changed from needs_review to needs_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 6 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:5 Changed 4 years ago by git

  • Commit changed from fddc853b27aaacd89d18ac1958742b343bb97ae7 to 07a1e2e464b4273c531ec6cccee89a42d13d8af1

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 4 years ago by saraedum

  • Dependencies changed from #16321 to #15692
Last edited 4 years ago by saraedum (previous) (diff)

comment:7 Changed 4 years ago by git

  • Commit changed from 07a1e2e464b4273c531ec6cccee89a42d13d8af1 to 0a788023afb9492f872f1da5cd44ce8a74c24f09

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 4 years ago by saraedum

  • Status changed from needs_work to needs_review

New commits:

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

comment:9 Changed 4 years ago by saraedum

  • Keywords days71 added

comment:10 Changed 4 years ago by roed

  • Branch changed from u/saraedum/ticket/16339 to u/roed/ticket/16339

comment:11 Changed 4 years ago by roed

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

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

comment:12 Changed 4 years ago by pbruin

  • Status changed from positive_review to needs_work

Merge conflict with #12603.

comment:13 Changed 4 years ago by aly.deines

  • Branch changed from u/roed/ticket/16339 to u/aly.deines/ticket/16339
  • Commit changed from 37499a44d839bd61a1920636fbbc68146b65c2f8 to 12e9c4379d64eec9e283ad1be3a356abd6afa8a6

Fixed merge conflict.

New commits:

​12e9c43Merged 16339 to develop.

comment:14 Changed 4 years ago by git

  • Commit changed from 12e9c4379d64eec9e283ad1be3a356abd6afa8a6 to c803a5f02d9925f98d603ddf8e72cb4d7496e8ec

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

​c803a5fFixed some imports.

comment:15 Changed 4 years ago by aly.deines

  • Status changed from needs_work to needs_review

comment:16 Changed 3 years ago by mderickx

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

  • Branch changed from u/aly.deines/ticket/16339 to u/saraedum/ticket/16339

comment:18 Changed 3 years ago by saraedum

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

  • Branch changed from u/aly.deines/ticket/16339 to u/saraedum/ticket/16339

comment:20 Changed 3 years ago by mderickx

  • Commit changed from c803a5f02d9925f98d603ddf8e72cb4d7496e8ec to 710ab456b3ec97e186ec5800fb3b5aac90e159d4

I would say yes unless the merge conflicts were nontrivial.

New commits:

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

comment:21 Changed 3 years ago by mderickx

  • Status changed from needs_review to positive_review

comment:22 Changed 3 years ago by vbraun

  • Branch changed from u/saraedum/ticket/16339 to 710ab456b3ec97e186ec5800fb3b5aac90e159d4
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.