#8998 closed defect (fixed)
galois_action on cusps has a bug
Reported by: | was | Owned by: | craigcitro |
---|---|---|---|
Priority: | major | Milestone: | sage-4.7 |
Component: | modular forms | Keywords: | |
Cc: | Merged in: | sage-4.7.alpha5 | |
Authors: | William Stein | Reviewers: | John Cremona |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
Ticket #5822 implemented the action of Galois on cusps. I think the algorithm was only designed to work for Gamma_0(N). However, the code runs for other groups, and doesn't raise an error. Unfortunately, it gives completely wrong results in some cases, e.g.,
sage: G = Gamma1(19) sage: rational_cusps = [c for c in G.cusps() if c.galois_action(2,19).is_gamma1_equiv(c,19)] sage: rational_cusps [0, 2/19, 1/9, 1/8, 1/7, 3/19, 1/6, 1/5, 4/19, 1/4, 5/19, 6/19, 1/3, 7/19, 8/19, 9/19, 1/2, Infinity]
However, exactly half the cusps are rational (see, e.g., my paper http://wstein.org/papers/j1p/ or the work of Kubert-Lang).
This came up in research that Michael Stoll and I were doing, and it was temporarily very confusing.
Attachments (1)
Change History (8)
Changed 10 years ago by
comment:1 Changed 10 years ago by
- Status changed from new to needs_review
comment:2 follow-up: ↓ 3 Changed 10 years ago by
Looks good -- testing now.
comment:3 in reply to: ↑ 2 Changed 10 years ago by
- Reviewers set to John Cremona
- Status changed from needs_review to positive_review
comment:4 Changed 10 years ago by
- Merged in set to sage-4.7.alpha5
- Resolution set to fixed
- Status changed from positive_review to closed
comment:5 Changed 9 years ago by
This ticket is a false ticket. I just looked in to the article and the code in the article is really general. The problem is that is_gamma1_equiv does not do what you expect it to!
if Cusp(0).is_gamma1_equiv(Cusp(infinity),101): print "You would not expect to see something printed"
Actually prints the above statement. The reason for this is that is_gamma1_equiv does not return a bool but a tuple. The first element of this tuple is the bool you were actually looking for.
By the way, I find the claim in this ticket that the code of 5882 only works for Gamma0(N) quite stupid because for Gamma0(n) one might as well implement the following:
def galois_action(self,t,N): return self
Since all cusps on X_0(N).
However there is still a bug in the code because:
N=5 G=Gamma1(N) for i in G.cusps(): print [j.galois_action(2,N).is_gamma1_equiv(i,N)[0] for j in G.cusps()].count(True)
Prints
2 1 0 1
So the the galois action is not even a permutation!
What would be the right thing to do? Reopen this ticket, open a new one?
comment:6 Changed 9 years ago by
Open a new one, with cross-referencing.
comment:7 Changed 9 years ago by
Ok, I created #13253 for this. And uploaded the patch I already created. It now needs review.
It turns out that cusps (P1(Q)) don't know their group of course. This function is thus somewhat badly named. A minimal invasive change without breaking backwards compatibility is to clarify the *documentation* and do nothing else. I think that is enough to close this ticket, though I may want to make another ticket later to add other actions, etc. But for now, it is certainly a big improvement to at least fix a major incorrect statement in the documentation! This patch is pretty safe, since it *only* changes documentation.