Fix doctest in sage/modular/overconvergent/hecke_series.py
Closed this issue · 27 comments
The goal of this ticket is to fix the bug I inadvertently introduced in #33876: the test:
sage: hecke_series(5, 7, 10000, 5, modformsring=True) # long time (3.4s)
never returns.
Component: modular forms
Author: David Ayotte
Branch: 9e83123
Reviewer: Volker Braun, Travis Scrimshaw, Guillermo Moreno-Socías
Issue created by migration from https://trac.sagemath.org/ticket/34025
I now consistently get this. I wonder if it comes from the new change_ring()? This is strange.
Upping the verbosity yields basically
verbose 1 (344: hecke_series.py, random_new_basis_modp) Calling echelonize mod 5.
verbose 1 (344: hecke_series.py, random_new_basis_modp) done with echelonize (time = 8.000000001118224e-06)
repeatedly.
Replying to @tscrim:
I now consistently get this. I wonder if it comes from the new
change_ring()? This is strange.Upping the verbosity yields basically
verbose 1 (344: hecke_series.py, random_new_basis_modp) Calling echelonize mod 5. verbose 1 (344: hecke_series.py, random_new_basis_modp) done with echelonize (time = 8.000000001118224e-06)repeatedly.
It's indeed very strange. I was able to fix the problem on my machine by reverting the line 240 of the filesage/modular/overconvergent/hecke_series.py:
- M = ModularFormsRing(N)
+ M = ModularFormsRing(N, base_ring=Zmod(p))I was under the impression that this base_ring parameter would not be important in the body of the method low_weight_generators, because in any case the gen_forms method returns a list of modular forms over Q.
Branch pushed to git repo; I updated commit sha1. New commits:
cef8bd2 | src/sage/modular/modform/ring.py: revert a deleted line of an alias |
Commit: cef8bd2
I reverted some changes I did in #33876, in order to fix this blocker quickly. The command
sage: hecke_series(5, 7, 10000, 5, modformsring=True) # long time (3.4s)
now runs as intended.
Branch pushed to git repo; I updated commit sha1. New commits:
9e83123 | src/sage/modular/overconvergent/hecke_series.py: fix failing doctest |
Reviewer: Volker Braun
Merge failure on top of:
c0abe3f Trac #33213: Replace SAGE_TMP by the system location in the sage library
7ab174a Trac #32340: document behavior of .is_prime() for number fields
c69f4af Trac #28263: Degree for Affine Morphism or Affine Dynamical System
e516392 Trac #34001: Add flag to avoid OrePolynomialRing cast to PolynomialRing
01b8bcb Trac #33993: pep cleanup for words/morphism.py
716c5ce Trac #33992: remove class inheritance of object in remaining places
de375bc Trac #33990: Subset_s _an_element_
0593e83 Trac #33987: modernize super() in structure, symbolic, doctest, databases
d9a45e2 Trac #33985: modernize super() in monoids,modules,modular
7ed4046 Trac #33984: modernize super() in schemes,libs,sets,quivers
1df260e Trac #33979: .roots() does not always return elements of the base ring
c6c2b85 Trac #33974: Documentation addition for symmetric functions - Cauchy identity
f92916e Trac #33840: bool(matrix) ignores exceptions raised while comparing entries
f8df808 Updated SageMath version to 9.7.beta3
author '' does not look right
Merge failure on top of:
c0abe3f Trac #33213: Replace SAGE_TMP by the system location in the sage library
7ab174a Trac #32340: document behavior of .is_prime() for number fields
c69f4af Trac #28263: Degree for Affine Morphism or Affine Dynamical System
e516392 Trac #34001: Add flag to avoid OrePolynomialRing cast to PolynomialRing
01b8bcb Trac #33993: pep cleanup for words/morphism.py
716c5ce Trac #33992: remove class inheritance of object in remaining places
de375bc Trac #33990: Subset_s _an_element_
0593e83 Trac #33987: modernize super() in structure, symbolic, doctest, databases
d9a45e2 Trac #33985: modernize super() in monoids,modules,modular
7ed4046 Trac #33984: modernize super() in schemes,libs,sets,quivers
1df260e Trac #33979: .roots() does not always return elements of the base ring
c6c2b85 Trac #33974: Documentation addition for symmetric functions - Cauchy identity
f92916e Trac #33840: bool(matrix) ignores exceptions raised while comparing entries
f8df808 Updated SageMath version to 9.7.beta3
author '' does not look right
Author: David Ayotte
Hello, where can I look for the merge failure? On my machine, I was able to merge the develop branch (v. 9.7.beta3) into my branch without any merge failure. I upgraded SageMath to version 9.7.beta3 and I'm currently building it.
Is this failure OS dependent?
On Apple Silicon, in an empty directory I did
git clone -c core.symlinks=true --branch develop --tags https://github.com/sagemath/sage.git
cd sage
git-trac-merge 34025 33213 32340 28263 34001 33993 33992 33990 33987 33985 33984 33979 33974 33840
make configure
./configure
source .homebrew-build-env
make
make pytest
make ptestlong
without any problems (apart from #33212 on macOS 11.6.7 with Xcode 13.2.1).
Output of merge attached.
Attachment: git-trac-merge 34025 33213 32340 28263 34001 33993 33992 33990 33987 33985 33984 33979 33974 33840.txt
Volker is saying there is a merge failure with one of those tickets. I am pretty sure it is #33985 with a trivial failure.
So I would just merge #33985 in, which perhaps your git merging is slightly smarter than what Volker is using (potentially to prevent any unintended changes from creeping in).
I am sorry, Travis, I am afraid I do not understand you.
My git-trac-merge is just
#!/bin/zsh
git remote add trac git://trac.sagemath.org/sage.git
for a in $*; do echo && echo '#'"$a" && git trac fetch $a && git merge --no-ff -m "Merge #$a" FETCH_HEAD; done
This is what I get:
% git-trac-merge 34025 33985
#34025
remote branch: u/gh-DavidAyotte/fix_blocker_in_hecke_series
From git://trac.sagemath.org/sage
* branch u/gh-DavidAyotte/fix_blocker_in_hecke_series -> FETCH_HEAD
* [new branch] u/gh-DavidAyotte/fix_blocker_in_hecke_series -> trac/u/gh-DavidAyotte/fix_blocker_in_hecke_series
Merge made by the 'recursive' strategy.
src/sage/modular/modform/ring.py | 27 ++++++++++++++-------------
src/sage/modular/overconvergent/hecke_series.py | 5 ++---
2 files changed, 16 insertions(+), 16 deletions(-)
#33985
remote branch: u/chapoton/33985
From git://trac.sagemath.org/sage
* branch u/chapoton/33985 -> FETCH_HEAD
* [new branch] u/chapoton/33985 -> trac/u/chapoton/33985
Auto-merging src/sage/monoids/indexed_free_monoid.py
Auto-merging src/sage/modules/tensor_operations.py
Auto-merging src/sage/modules/multi_filtered_vector_space.py
Auto-merging src/sage/modules/filtered_vector_space.py
Auto-merging src/sage/modular/modform_hecketriangle/graded_ring_element.py
Merge made by the 'recursive' strategy.
src/sage/modular/arithgroup/arithgroup_generic.py | 2 +-
src/sage/modular/btquotients/btquotient.py | 6 +++---
src/sage/modular/btquotients/pautomorphicform.py | 12 ++++++------
src/sage/modular/dirichlet.py | 5 ++---
src/sage/modular/hecke/algebra.py | 4 +---
src/sage/modular/modform_hecketriangle/abstract_ring.py | 2 +-
src/sage/modular/modform_hecketriangle/abstract_space.py | 5 ++---
src/sage/modular/modform_hecketriangle/analytic_type.py | 18 +++++++-----------
src/sage/modular/modform_hecketriangle/element.py | 6 +++---
src/sage/modular/modform_hecketriangle/graded_ring_element.py | 13 +++++--------
src/sage/modular/modform_hecketriangle/hecke_triangle_groups.py | 4 ++--
src/sage/modular/modform_hecketriangle/series_constructor.py | 7 +++----
src/sage/modular/modform_hecketriangle/subspace.py | 15 +++++----------
src/sage/modules/fg_pid/fgp_module.py | 2 +-
src/sage/modules/filtered_vector_space.py | 2 +-
src/sage/modules/fp_graded/free_module.py | 8 ++++----
src/sage/modules/fp_graded/module.py | 4 ++--
src/sage/modules/free_module.py | 12 ++++++------
src/sage/modules/multi_filtered_vector_space.py | 2 +-
src/sage/modules/tensor_operations.py | 4 ++--
src/sage/modules/torsion_quadratic_module.py | 2 +-
src/sage/modules/vector_numpy_dense.pyx | 3 +--
src/sage/modules/with_basis/cell_module.py | 4 ++--
src/sage/modules/with_basis/invariant.py | 6 +++---
src/sage/modules/with_basis/representation.py | 3 ++-
src/sage/modules/with_basis/subquotient.py | 5 ++---
src/sage/monoids/automatic_semigroup.py | 3 ++-
src/sage/monoids/free_monoid.py | 2 +-
src/sage/monoids/indexed_free_monoid.py | 4 ++--
src/sage/monoids/trace_monoid.py | 2 +-
30 files changed, 75 insertions(+), 92 deletions(-)
%
Replying to @GMS103:
I am sorry, Travis, I am afraid I do not understand you.
Actually, I was wrong. #33985 touches different files than #34025. I didn't look closely enough. I think the secret to the "merge failure" is actually the last line
author does not look right
Which did not have David listed as the author at the time. So the message Volker copied in is slightly misleading in that regard. I think we can just set this back to a positive review (especially given that you checked it merged cleaning and none of the other tickets touch the same files).
Changed reviewer from Volker Braun to Volker Braun, Travis Scrimshaw, Guillermo Moreno-Socías
Thank you all for investigating this, I was a little bit confused about this and my SageMath was currently building.
Changed branch from u/gh-DavidAyotte/fix_blocker_in_hecke_series to 9e83123
This is perhaps a stupid question, but do closed tickets get merged in the develop branch?
I ask because with the current develop branch as is, the test fixed by this ticket fails in exactly the same way.
Yes, but only when the next beta is released (note the top commit in develop is always a beta).
Thanks, Travis.