Opened 5 years ago

Closed 5 years ago

#19060 closed defect (fixed)

Fix gauss_sum() for Dirichlet characters over cyclotomic fields

Reported by: pbruin Owned by:
Priority: major Milestone: sage-6.9
Component: number theory Keywords: Dirichlet character Gauss sum
Cc: Merged in:
Authors: Peter Bruin Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: ad61452 (Commits) Commit: ad6145286b8bae19ac6f3f7d7078a7cb8b4dd41b
Dependencies: #19056 Stopgaps:

Description

In SageMath 6.9.beta2: 

sage: K.<z> = CyclotomicField(8)
sage: G = DirichletGroup(13, K)
sage: chi = G([z^2])
sage: chi.gauss_sum()
Traceback (most recent call last):
...
TypeError: Cannot coerce zeta52 into Cyclotomic Field of order 8 and degree 4

This arises because chi is actually defined over a smaller cyclotomic field. It can be fixed by letting gauss_sum() minimize the base ring.

Change History (17)

comment:1 Changed 5 years ago by pbruin

  • Branch set to u/pbruin/19060-gauss_sum_cyclotomic_field
  • Commit set to 6ea900bbe4ea86533daacdadec083a3f2db5c893
  • Status changed from new to needs_review

comment:2 Changed 5 years ago by git

  • Commit changed from 6ea900bbe4ea86533daacdadec083a3f2db5c893 to 95b5401cf55f8650dfcc222d32da081c32ba1044

Branch pushed to git repo; I updated commit sha1. New commits:

​95b5401Trac 19060: fix doctests

comment:3 Changed 5 years ago by git

  • Commit changed from 95b5401cf55f8650dfcc222d32da081c32ba1044 to c5f0a8a1e5186d160c22f933a97b9d41b82bc70b

Branch pushed to git repo; I updated commit sha1. New commits:

​ca77b27Trac 19056: implement gauss_sum and gauss_sum_numerical for Dirichlet characters over QQbar
​c5f0a8aMerge branch 'ticket/19056-gauss_sum_algebraic_field' into ticket/19060-gauss_sum_cyclotomic_field

comment:4 Changed 5 years ago by pbruin

  • Dependencies set to #19056

Added #19056 as a dependency because it has positive review and there is a merge conflict.

comment:5 Changed 5 years ago by chapoton

  • Status changed from needs_review to needs_work

two failing doctest, problem of complex conjugate

comment:6 Changed 5 years ago by git

  • Commit changed from c5f0a8a1e5186d160c22f933a97b9d41b82bc70b to 4236cdce5113b9a732db96b21cf7f9d38d1c7682

Branch pushed to git repo; I updated commit sha1. New commits:

​4236cdcTrac 19060: make doctests less sensitive to numerical noise

comment:7 Changed 5 years ago by pbruin

  • Status changed from needs_work to needs_review

comment:8 follow-up: Changed 5 years ago by chapoton

Hmm. Precision seems too high to me.. Maybe 1e-15 would be better ?

and maybe replace the very small imaginary part in the doctest by 0.0*I ?

comment:9 Changed 5 years ago by git

  • Commit changed from 4236cdce5113b9a732db96b21cf7f9d38d1c7682 to 4cd66fddb6fd862b665c78d95e6b506b79ac201e

Branch pushed to git repo; I updated commit sha1. New commits:

​4cd66fdTrac 19060: increase doctest tolerance

comment:10 in reply to: ↑ 8 Changed 5 years ago by pbruin

Replying to chapoton:

Hmm. Precision seems too high to me.. Maybe 1e-15 would be better ?

You are right, done.

and maybe replace the very small imaginary part in the doctest by 0.0*I ?

I prefer to have the doctest output reflect the actual output (on some system) as closely as possible, and I have never seen 0.0*I in these tests (of course it would be the ideal answer).

comment:11 Changed 5 years ago by chapoton

  • Status changed from needs_review to needs_work

one failing doctest, need to relax the tolerance a little bit, see patchbot report

comment:12 Changed 5 years ago by git

  • Commit changed from 4cd66fddb6fd862b665c78d95e6b506b79ac201e to ad6145286b8bae19ac6f3f7d7078a7cb8b4dd41b

Branch pushed to git repo; I updated commit sha1. New commits:

​ad61452Trac 19060: further increase doctest tolerance

comment:13 Changed 5 years ago by pbruin

  • Status changed from needs_work to needs_review

comment:14 Changed 5 years ago by chapoton

  • Status changed from needs_review to positive_review

ok, looks good to me

comment:15 Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_work

Reviewer name missing

comment:16 Changed 5 years ago by chapoton

  • Reviewers set to Frédéric Chapoton
  • Status changed from needs_work to positive_review

sorry for that.

comment:17 Changed 5 years ago by vbraun

  • Branch changed from u/pbruin/19060-gauss_sum_cyclotomic_field to ad6145286b8bae19ac6f3f7d7078a7cb8b4dd41b
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.