Opened 7 years ago

Closed 7 years ago

#19954 closed enhancement (fixed)

QQbar cleaning 1

Reported by: Vincent Delecroix 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 Vincent Delecroix)

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 7 years ago by Vincent Delecroix

Description: modified (diff)

comment:2 Changed 7 years ago by Vincent Delecroix

Branch: u/vdelecroix/19954
Commit: 7ab3ffbfd3e824c73a1c78d59de84b37f4b94e2c
Status: newneeds_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 7 years ago by Frédéric Chapoton

Status: needs_reviewneeds_work

failing doctests, see patchbot report

comment:4 Changed 7 years ago by git

Commit: 7ab3ffbfd3e824c73a1c78d59de84b37f4b94e2cd1788a00924a77f218bbff8c74d0eb56a7afa55c

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

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

comment:5 Changed 7 years ago by Vincent Delecroix

Status: needs_workneeds_review

comment:6 Changed 7 years ago by Frédéric Chapoton

Still one doctest failing

comment:7 Changed 7 years ago by git

Commit: d1788a00924a77f218bbff8c74d0eb56a7afa55c94629dbef9c590f0f748d1c0e40e5833f226c245

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

94629dbTrac 19954: doctest independent of execution order

comment:8 Changed 7 years ago by Vincent Delecroix

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

comment:9 Changed 7 years ago by Frédéric 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 7 years ago by Vincent Delecroix

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 7 years ago by git

Commit: 94629dbef9c590f0f748d1c0e40e5833f226c245cb2592cf73f815ccb611babbd18a3be19e95b235

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

cb2592cTrac 19954: typo in documentation

comment:12 Changed 7 years ago by Frédéric 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 7 years ago by git

Commit: cb2592cf73f815ccb611babbd18a3be19e95b235c1f5c5fb96bf5bb06c4dacdaf132d7ce2b3c8de3

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

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

comment:14 Changed 7 years ago by Vincent Delecroix

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

comment:15 Changed 7 years ago by Vincent Delecroix

Description: modified (diff)

comment:16 Changed 7 years ago by Frédéric Chapoton

Reviewers: Frédéric Chapoton
Status: needs_reviewpositive_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 7 years ago by Vincent Delecroix

Thanks Frédéric!

comment:18 Changed 7 years ago by Volker Braun

Branch: u/vdelecroix/19954c1f5c5fb96bf5bb06c4dacdaf132d7ce2b3c8de3
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.