Opened 6 years ago

Closed 6 years ago

#19954 closed enhancement (fixed)

QQbar cleaning 1

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-7.1
Component: number fields Keywords:
Cc: Merged in:
Authors: Vincent Delecroix Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: c1f5c5f (Commits, GitHub, GitLab) Commit: c1f5c5fb96bf5bb06c4dacdaf132d7ce2b3c8de3
Dependencies: Stopgaps:

Status badges

Description (last modified by vdelecroix)

This is a series of ticket about QQbar cleaning. In this first one we:

  • get rid of ANRootOfUnity descriptor
  • get rid of special casing for 'zero', 'rootunity', 'imaginary'
  • move the 34-gon commented function as a doctest

By doing so we slow down the code for minpoly but this will be fixed in the subsequent ticket #19955.

This ticket is part of the task #18333. See also the follow up #20074.

Change History (18)

comment:1 Changed 6 years ago by vdelecroix

  • Description modified (diff)

comment:2 Changed 6 years ago by vdelecroix

  • Branch set to u/vdelecroix/19954
  • Commit set to 7ab3ffbfd3e824c73a1c78d59de84b37f4b94e2c
  • Status changed from new to needs_review

New commits:

be379c1Trac 19954: get rid of ANRootOfUnity
9df5428Trac 19954: get rid of the argument _is_pow for ANRoot
b35d6f6Trac 19954: fix doctests
9f39b58Trac 19954: fix sage_input doctests
7ab3ffbTrac 19954: move the 34-gon as a doctest

comment:3 Changed 6 years ago by chapoton

  • Status changed from needs_review to needs_work

failing doctests, see patchbot report

comment:4 Changed 6 years ago by git

  • Commit changed from 7ab3ffbfd3e824c73a1c78d59de84b37f4b94e2c to d1788a00924a77f218bbff8c74d0eb56a7afa55c

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

ec2e9aaTrac 19954: merge 7.1.beta1
d1788a0Trac 19954: fix doctests

comment:5 Changed 6 years ago by vdelecroix

  • Status changed from needs_work to needs_review

comment:6 Changed 6 years ago by chapoton

Still one doctest failing

comment:7 Changed 6 years ago by git

  • Commit changed from d1788a00924a77f218bbff8c74d0eb56a7afa55c to 94629dbef9c590f0f748d1c0e40e5833f226c245

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

94629dbTrac 19954: doctest independent of execution order

comment:8 Changed 6 years ago by vdelecroix

I see. The doctest was dependent of execution order. I changed for a more neutral one.

comment:9 Changed 6 years ago by chapoton

I have found a typo:

Class to upport old unpickling

And I am a bit puzzled by the very complicated shape of the new results of the sage_input method.

comment:10 Changed 6 years ago by vdelecroix

The sage_input is just intended to reconstruct the object. It is the very same with

sage: A = identity_matrix(4)
sage: sage_input(A)
matrix(ZZ, [[1, 0, 0, 0], [0, 1, 0, 0], [0, 0, 1, 0], [0, 0, 0, 1]])

the answer is hopefully not identity_matrix(4)!

Before this ticket, the algebraic numbers used to remember the way they were constructed. Which is a complete nonsense.

comment:11 Changed 6 years ago by git

  • Commit changed from 94629dbef9c590f0f748d1c0e40e5833f226c245 to cb2592cf73f815ccb611babbd18a3be19e95b235

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

cb2592cTrac 19954: typo in documentation

comment:12 Changed 6 years ago by chapoton

the description at the top of the documentation still refers to

Algebraic numbers exist in one of the following forms:

a rational number
the product of a rational number and an n‘th root of unity
etc

The second line should be removed, no ?

comment:13 Changed 6 years ago by git

  • Commit changed from cb2592cf73f815ccb611babbd18a3be19e95b235 to c1f5c5fb96bf5bb06c4dacdaf132d7ce2b3c8de3

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

1385753Trac 19954: documentation
c1f5c5fTrac 19954: remove useless "gaussian" functions

comment:14 Changed 6 years ago by vdelecroix

Right! I also remove functions relative to "gaussian" which no longer exists.

comment:15 Changed 6 years ago by vdelecroix

  • Description modified (diff)

comment:16 Changed 6 years ago by chapoton

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

ok, this seems to be good to go.

One can note that there is the UniversalCyclotomicField for people only caring about the abelian closure of QQ.

comment:17 Changed 6 years ago by vdelecroix

Thanks Frédéric!

comment:18 Changed 6 years ago by vbraun

  • Branch changed from u/vdelecroix/19954 to c1f5c5fb96bf5bb06c4dacdaf132d7ce2b3c8de3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.