Ticket #4359 (closed defect: fixed)

Opened 5 years ago

Last modified 5 years ago

[with patch, positive review] Huge number of small fixes to modular forms code

Reported by: craigcitro Owned by: craigcitro
Priority: major Milestone: sage-3.2
Component: modular forms Keywords:
Cc: Work issues:
Report Upstream: Reviewers:
Authors: Merged in:
Dependencies: Stopgaps:

Description

This is just a big bundle of fixes to the modular forms code that I had piled up.

Attachments

trac-4359.patch Download (20.3 KB) - added by craigcitro 5 years ago.
trac-4359-2.patch Download (870 bytes) - added by craigcitro 5 years ago.

Change History

Changed 5 years ago by craigcitro

comment:1 Changed 5 years ago by AlexGhitza

Looks good. I have some questions about _ensure_is_compatible() in modform/element.py

  1. I guess I don't quite know what the function is meant to be used for; the docstring says "compatible for arithmetic and comparison operations". I assume arithmetic here means addition or subtraction?
  1. With the patch, two forms of the same weight but different groups of the same level are deemed compatible. For instance, if f is on Gamma0(11) and g is on Gamma1(11), or if f and g are on Gamma1(17) but with different Dirichlet characters. Is this the desired behaviour?

Changed 5 years ago by craigcitro

comment:2 Changed 5 years ago by craigcitro

  • Status changed from new to assigned

Ah, good point. I added a patch that changes it to test that they have the same ambient space, which is what the docstring claims.

comment:3 Changed 5 years ago by AlexGhitza

  • Summary changed from [with patch, needs review] Huge number of small fixes to modular forms code to [with patch, positive review] Huge number of small fixes to modular forms code

OK, I'm happy.

comment:4 Changed 5 years ago by mabshoff

  • Status changed from assigned to closed
  • Resolution set to fixed

Merged both patches in Sage 3.2.alpha1

Note: See TracTickets for help on using tickets.