#7598 closed defect (fixed)
NumberField embedding slightly off
Reported by: | mhansen | Owned by: | davidloeffler |
---|---|---|---|
Priority: | major | Milestone: | sage-4.3 |
Component: | number fields | Keywords: | |
Cc: | burcin | Merged in: | sage-4.3.rc1 |
Authors: | William Stein | Reviewers: | Mike Hansen, John Cremona |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
sage: Q.<i> = NumberField(x^2+1) sage: complex(i) 0.99999999999999967j
It should give 1j
instead.
Attachments (3)
Change History (18)
comment:1 Changed 10 years ago by
- Milestone set to sage-4.3
comment:2 Changed 10 years ago by
- Cc burcin added
comment:3 Changed 10 years ago by
Changed 10 years ago by
comment:4 Changed 10 years ago by
trac_7598-more_serious_version.patch -- this deals with the problems more at the root. Unfortunately, there are doctests in this file that fail:
sage -t devel/sage-main/sage/modular/dirichlet.py # 4 doctests failed
and I haven't had time to figure out what is wrong. It probably has to do with a complex embedding not being defined automatically, whereas before it was...
The design of embeddings was really bad before and relied on numerical errors to mess up the order of roots in case of 53 bit precision. This was potentially *very* buggy and was I think the result of some absolutely terrible design decisions. This absolutely must be fixed before releasing sage-4.3. This patch basically fixes it, modulo some small remaining issue.
Here is an example from sage-4.2.1 that illustrates just how horrendously bad the previous design was (with using CDF when prec=53 but ComplexField?(prec) otherwise):
sage: K.<i> = QuadraticField(-1) sage: i.complex_em i.complex_embedding i.complex_embeddings sage: i.complex_embedding() 1.0*I sage: i.complex_embedding(100) -1.0000000000000000000000000000*I
comment:5 Changed 10 years ago by
I think the errors in dirichlet.py comes from the following:
sage: a = mod(1,3) sage: CDF.zeta(3)^a -0.5 + 0.866025403784*I sage: CC.zeta(3)^a --------------------------------------------------------------------------- Traceback (most recent call last) ... TypeError: unable to coerce <type 'sage.rings.complex_number.ComplexNumber'> to an integer
It'd be nice if the {{{pow}} methods were standardized.
Changed 10 years ago by
comment:6 Changed 10 years ago by
- Reviewers set to Mike Hansen
- Status changed from new to needs_review
I've added a patch which fixes the errors in dirichlet.py by just forcing the exponent to be an int.
William's changes look good to me so the only thing that needs review is trac_7598-dirichlet.patch.
We should open another ticket for the __pow__
discrepencies.
comment:7 Changed 10 years ago by
Do we need to apply both patches? Or is it that Mike has given a positive review to the first and only needs a review of the second? I was hoping to test this (being someone who experienced the problems) but as it will take a long time to rebuild, I want to make sure that the time is not wasted.
comment:8 Changed 10 years ago by
You need to apply both patches. I've given a positive review to the first. The second is pretty easy since it makes the exponent an integer instead of an integermod.
comment:9 Changed 10 years ago by
- Status changed from needs_review to needs_work
Running long doctests on a 32-bit Arch Linux machine gives one doctest failure related to this ticket:
sage -t -long "devel/sage/doc/en/bordeaux_2008/nf_galois_groups.rst" ********************************************************************** File "/opt/sage-4.3.rc0/devel/sage/doc/en/bordeaux_2008/nf_galois_groups.rst", line 109: sage: K.complex_embeddings() Expected: [ Ring morphism: From: Number Field in a with defining polynomial x^3 - 2 To: Complex Double Field Defn: a |--> -0.629960524947 - 1.09112363597*I, Ring morphism: From: Number Field in a with defining polynomial x^3 - 2 To: Complex Double Field Defn: a |--> -0.629960524947 + 1.09112363597*I, Ring morphism: From: Number Field in a with defining polynomial x^3 - 2 To: Complex Double Field Defn: a |--> 1.25992104989 ] Got: [ Ring morphism: From: Number Field in a with defining polynomial x^3 - 2 To: Complex Field with 53 bits of precision Defn: a |--> -0.629960524947437 - 1.09112363597172*I, Ring morphism: From: Number Field in a with defining polynomial x^3 - 2 To: Complex Field with 53 bits of precision Defn: a |--> -0.629960524947437 + 1.09112363597172*I, Ring morphism: From: Number Field in a with defining polynomial x^3 - 2 To: Complex Field with 53 bits of precision Defn: a |--> 1.25992104989487 ] ********************************************************************** 1 items had failures: 1 of 4 in __main__.example_3 ***Test Failed*** 1 failures. For whitespace errors, see the file /home/ghitza/.sage//tmp/.doctest_nf_galois_groups.py [5.9 s] exit code: 1024
Apart from this small issue, everything looks good.
comment:10 Changed 10 years ago by
OK, I've added a trivial patch that fixes the last doctest failure.
comment:11 Changed 10 years ago by
- Status changed from needs_work to needs_review
comment:12 Changed 10 years ago by
- Reviewers changed from Mike Hansen to Mike Hansen, John Cremona
- Status changed from needs_review to positive_review
Applied all 3 patches to 4.3.rc0 on both 32-bit Suse (built using system gfortran) and 64-bit ubuntu (using Sage's gfortran). No problems. All tests in the files touched pass. I still get a failure in doc/en/bordeaux_2008/nf_introduction.rst but that has a different cause.
So, positive review.
comment:13 Changed 10 years ago by
Excellent. I'm glad we (you guys) tracked that down. I'm not sure why I didn't think that the number field was just using the alternate embedding.
comment:14 Changed 10 years ago by
- Merged in set to sage-4.3.rc1
- Resolution set to fixed
- Status changed from positive_review to closed
comment:15 Changed 10 years ago by
- Milestone changed from sage-4.3.1 to sage-4.3
This is probably caused by the roots method using NumPy? which uses Fortran which is a little off. If you specify algorithm='pari' to the roots command when computing them, things should be fine.
Note that NumPy? is not used for the second example.