Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with cuspidal_subspace and new_subspace for modular symbols #2535

Closed
craigcitro opened this issue Mar 15, 2008 · 8 comments
Closed

Comments

@craigcitro
Copy link
Member

There's some error with plus_submodule and cuspidal_submodule not being "commutative." Here's an example:

sage: M = ModularSymbols(11,2)
sage: Mpc = M.plus_submodule().cuspidal_submodule()
sage: Mcp = M.cuspidal_submodule().plus_submodule()

sage: Mcp.q_expansion_basis(10) 
[
q - 2*q^2 - q^3 + 2*q^4 + q^5 + 2*q^6 - 2*q^7 - 2*q^9 + O(q^10)
]

sage: Mpc.q_expansion_basis(10)
---------------------------------------------------------------------------
<type 'exceptions.RuntimeError'>          Traceback (most recent call last)

/Users/craigcitro/<ipython console> in <module>()

/sage/local/lib/python2.5/site-packages/sage/modular/modsym/space.py in q_expansion_basis(self, prec, algorithm)
    458             algorithm = 'hecke'
    459         if algorithm == 'hecke':
--> 460             return Sequence(self._q_expansion_basis_hecke_dual(prec), cr=True)
    461         elif algorithm == 'eigen':
    462             return Sequence(self._q_expansion_basis_eigen(prec), cr=True)


/sage/local/lib/python2.5/site-packages/sage/modular/modsym/space.py in _q_expansion_basis_hecke_dual(self, prec)
    913         t = misc.verbose('computing basis to precision %s'%prec)
    914         while V.dimension() < d and i >= 0:
--> 915             v = [self.dual_hecke_matrix(n).column(i) for n in range(1,prec)]
    916             t = misc.verbose('iteration: %s'%j,t)
    917             X = M(v).transpose()

/sage/local/lib/python2.5/site-packages/sage/modular/hecke/module.py in dual_hecke_matrix(self, n)
    725             self._dual_hecke_matrices = {}
    726         if not self._dual_hecke_matrices.has_key(n):
--> 727             T = self._compute_dual_hecke_matrix(n)
    728             self._dual_hecke_matrices[n] = T
    729         return self._dual_hecke_matrices[n]

/sage/local/lib/python2.5/site-packages/sage/modular/hecke/submodule.py in _compute_dual_hecke_matrix(self, n)
    108         A = self.ambient_hecke_module().dual_hecke_matrix(n)
    109         check =  arith.gcd(self.level(), n) != 1
--> 110         return A.restrict(self.dual_free_module(), check=check)
    111 
    112     def _compute_hecke_matrix(self, n):

/sage/local/lib/python2.5/site-packages/sage/modular/hecke/submodule.py in dual_free_module(self, bound, anemic)
    295                 # failed
    296                 raise RuntimeError, "Computation of embedded dual vector space failed " + \
--> 297                       "(cut down to rank %s, but should have cut down to rank %s)."%(V.rank(), self.rank())
    298 
    299 

<type 'exceptions.RuntimeError'>: Computation of embedded dual vector space failed (cut down to rank 2, but should have cut down to rank 1).

I'll look at this soon.

CC: @aghitza @JohnCremona

Component: modular forms

Issue created by migration from https://trac.sagemath.org/ticket/2535

@craigcitro craigcitro added this to the sage-3.4.1 milestone Mar 15, 2008
@craigcitro craigcitro self-assigned this Mar 15, 2008
@craigcitro
Copy link
Member Author

comment:1

The underlying bug was that we didn't use the star operator in addition to the Hecke operators when trying to determine the dual space to a space of modular symbols. I added this in, though there are several choices as to how to best do that. I added in at least two of these, and added a flag that chooses between them. I tested a handful of small examples, and picked the clear winner on these examples as a default. We should revisit this at some point and decide if that's really the best choice, or if there are tradeoffs with weight vs. level, etc.

I also added a few doctests here and there, since I was at it.

@ncalexan
Copy link
Mannequin

ncalexan mannequin commented Apr 11, 2008

comment:2

Patch looks good -- code is clean, doctests are good.

I cannot be the only judge on this patch because I am not expert in the functionality.

One change I would advocate is documenting the choices for use_sign. See the source is not the best documentation :)

@ncalexan ncalexan mannequin changed the title Problem with cuspidal_subspace and new_subspace for modular symbols [with partial positive review] Problem with cuspidal_subspace and new_subspace for modular symbols Apr 11, 2008
@williamstein
Copy link
Contributor

comment:3

Negative review, since the code doesn't work on the example given below:

17:47 < wstein-2535> craigcitro -- what happens with this new code when the submodule we're trying to get the
17:47 < wstein-2535> dual of has both * eigenvalues?
17:48 < wstein-2535> I think this new code might just break then.
17:48 -!- roed_ [n=roed@c-98-216-48-4.hsd1.ma.comcast.net] has quit []
17:48 < craigcitro> ah, you're saying i want an 'else' clause in the 'star' case.
17:48 < wstein-2535>  At a minimum.
17:49 < wstein-2535> And also you *have* to use star in that case.
17:49 < wstein-2535> Or something.
17:49 < wstein-2535> You've only treated one cases, namely * = constant on subspace.
17:49 < wstein-2535> But this "use the *" trick should work more generaly for any *-equivariant module.
17:49 < craigcitro> oh, you're saying in that case, take the + and - submodules and then just take the sum
17:50 < craigcitro> (i.e. do the same thing i'd do in the case of one eigenvalue with each of them)
17:50 < wstein-2535> sage: M = ModularSymbols(43).cuspidal_submodule()
17:50 < wstein-2535> sage: S = M[0].plus_submodule() + M[1].minus_submodule()
17:50 < wstein-2535> sage: S.dual_free_module()
17:50 < wstein-2535> boom!
17:50 < wstein-2535> Yes, exactly.

@williamstein williamstein changed the title [with partial positive review] Problem with cuspidal_subspace and new_subspace for modular symbols [with partial positive and partial negative review] Problem with cuspidal_subspace and new_subspace for modular symbols Apr 12, 2008
@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Sep 20, 2008

comment:5

Reclassify so this ticket is picked up properly by the various reports.

Cheers,

Michael

@sagetrac-mabshoff sagetrac-mabshoff mannequin changed the title [with partial positive and partial negative review] Problem with cuspidal_subspace and new_subspace for modular symbols Problem with cuspidal_subspace and new_subspace for modular symbols Sep 20, 2008
@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Nov 2, 2008

comment:7

Could #1127 be related?

Cheers,

Michael

@aghitza
Copy link

aghitza commented Jan 22, 2009

comment:8

The attached patch addresses William's counterexample, and should be applied after Craig's patch.

@craigcitro
Copy link
Member Author

comment:9

Attachment: trac-2535-final.patch.gz

Final version of patch attached, with one or two small improvements over previous.

@sagetrac-mabshoff
Copy link
Mannequin

sagetrac-mabshoff mannequin commented Jan 24, 2009

comment:11

Merged in Sage 3.3.alpha2

Cheers,

Michael

@sagetrac-mabshoff sagetrac-mabshoff mannequin modified the milestones: sage-3.4.1, sage-3.3 Jan 24, 2009
@sagetrac-mabshoff sagetrac-mabshoff mannequin closed this as completed Jan 24, 2009
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants