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

Reducing Gamma(N) cusps does not always reduce #36163

Closed
2 tasks done
JohnCremona opened this issue Aug 31, 2023 · 0 comments · Fixed by #36165
Closed
2 tasks done

Reducing Gamma(N) cusps does not always reduce #36163

JohnCremona opened this issue Aug 31, 2023 · 0 comments · Fixed by #36165

Comments

@JohnCremona
Copy link
Member

JohnCremona commented Aug 31, 2023

Steps To Reproduce

In Sage-10.1:

sage: Gamma(7).reduce_cusp(Cusp(6,7))
6/7
sage: Gamma(7).reduce_cusp(Cusp(1,0))
Infinity
sage: Gamma(7).are_equivalent(Cusp(1,0), Cusp(6,7))
True

Expected Behavior

Both 6/7 and 1/0 should reduce to Infinity = 1/0 since 6/7 and 1/0 are equivalent modulo Gamma(7), and according to the definition of a reduced cusp for Gamma(N). So we should see

sage: Gamma(7).reduce_cusp(Cusp(6,7))
Infinity

Actual Behavior

instead of

sage: Gamma(7).reduce_cusp(Cusp(6,7))
6/7

Additional Information

The code reduces both u and v mod N (where the cusp is u/v and the level is N) and then calls the hidden function _lift_pair(u,v,N). But that function assumes that its input u,v are coprime, which in this case they are not.

To fix this it suffices to change _lift_pair() to return Cusp(1,0), i.e. Infinity, when v%N==0 and u%N is either +1 or N-1; the second case was being missed.

Environment

- **OS**: Ubuntu 22.04.3 LTS
- **Sage Version**: 10.1

Checklist

  • I have searched the existing issues for a bug report that matches the one I want to file, without success.
  • I have read the documentation and troubleshoot guide
@JohnCremona JohnCremona self-assigned this Aug 31, 2023
JohnCremona added a commit to JohnCremona/sage that referenced this issue Aug 31, 2023
vbraun pushed a commit to vbraun/sage that referenced this issue Sep 14, 2023
    
The method reduce_cusp for Gamma(N) did not work properly on cusps u/v
where (u,v) is congruent to (-1,0) mod N.  This fixes that by fixing the
function _lift_pair().  A doctest has been added with the example
reported on sage-devel.

Fixes sagemath#36163

### 📝 Checklist

- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] I have created tests covering the changes.
- [x] I have updated the documentation accordingly.
    
URL: sagemath#36165
Reported by: John Cremona
Reviewer(s): Frédéric Chapoton
@vbraun vbraun closed this as completed in 4f4fbc2 Sep 16, 2023
@mkoeppe mkoeppe added this to the sage-10.2 milestone Sep 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants