Opened 10 years ago

Closed 9 years ago

#13200 closed enhancement (fixed)

Add a function for computing Dedekind sums

Reported by: fredrik.johansson Owned by: was
Priority: major Milestone: sage-5.11
Component: number theory Keywords:
Cc: Merged in: sage-5.11.beta2
Authors: Fredrik Johansson Reviewers: Frédéric Chapoton, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #13199 Stopgaps:

Status badges

Description (last modified by tscrim)

The patch adds dedekind_sum() to arith.py, wrapping the FLINT implementation.

Pari is also exposed (inefficiently, using the gp interface) for reference purposes. Note that FLINT and Pari return different results when gcd(p,q) != 1. The Dedekind is usually simply not defined in this case. If anyone knows a use for Dedekind sums with gcd(p,q) != 1, and a reason to prefer one definition over the other, I would be interested in hearing about it.

We could, alternatively, check that gcd(p,q) = 1 and otherwise return 0 or raise an exception (this would add a bit of overhead).

The patch depends on #13199 for a library file added there.


Apply:

Attachments (3)

dedekind_sum.patch (5.1 KB) - added by fredrik.johansson 10 years ago.
trac_13200_dedekind_sum_v2.patch (5.3 KB) - added by chapoton 9 years ago.
trac_13200-dedekind_sum-review-ts.patch (3.0 KB) - added by tscrim 9 years ago.

Download all attachments as: .zip

Change History (13)

Changed 10 years ago by fredrik.johansson

comment:1 Changed 10 years ago by chapoton

  • Dependencies changed from 13199 to #13199

comment:2 Changed 10 years ago by chapoton

  • Status changed from new to needs_review

Changed 9 years ago by chapoton

comment:3 Changed 9 years ago by chapoton

for the bot:

apply trac_13200_dedekind_sum_v2.patch

here is rebased patch, that should pass all tests.

Changed 9 years ago by tscrim

comment:4 Changed 9 years ago by tscrim

  • Reviewers set to Frédéric Chapoton, Travis Scrimshaw

Hey Frederic,

I've uploaded a review patch which does some minor tweaks. If you're happy with my changes, feel free to set this to positive review.

Best,
Travis

comment:5 Changed 9 years ago by chapoton

ok, positive review

comment:6 Changed 9 years ago by chapoton

  • Status changed from needs_review to positive_review

comment:7 Changed 9 years ago by jdemeyer

  • Status changed from positive_review to needs_info

Which patch(es) should be applied?

comment:8 Changed 9 years ago by tscrim

  • Description modified (diff)
  • Status changed from needs_info to needs_review

Sorry about that. Here they are.

Apply: trac_13200_dedekind_sum_v2.patch, trac_13200-dedekind_sum-review-ts.patch

comment:9 Changed 9 years ago by tscrim

  • Status changed from needs_review to positive_review

comment:10 Changed 9 years ago by jdemeyer

  • Merged in set to sage-5.11.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.