Add a __float__ method to the class Universal Cyclotomic Field

[sagetrac-vripoll]

There is currently no __float__ method for the class Universal Cyclotomic Field. For example:

    sage: UCF.<E> = UniversalCyclotomicField()
    sage: float(E(5)+E(5)^(-1))
    ---------------------------------------------------------------------------
    TypeError                                 Traceback (most recent call last)
    <ipython-input-2-0d63f957cb28> in <module>()
    ----> 1 float(E(Integer(5))+E(Integer(5))**(-Integer(1)))

    TypeError: float() argument must be a string or a number
    
    sage: float(CDF(E(5)+E(5)^(-1)).real_part())
    0.6180339887498949

We would like something such as:

    sage: float(E(5)+E(5)^(-1))
    0.6180339887498949

This ticket is a prerequisite to #15703.

CC: @jplab @stumpc5 @sagetrac-sage-combinat @sagetrac-tmonteil

Component: number fields

Keywords: Cyclotomic field, float, days57

Author: Vivien Ripoll

Branch/Commit: e5e01d0

Reviewer: Frédéric Chapoton, Marc Mezzarobba, Thierry Monteil

Issue created by migration from https://trac.sagemath.org/ticket/16120

[sagetrac-vripoll]

Branch: u/vripoll/ticket16120

[stumpc5]

Commit: d025f0f

[stumpc5]

comment:2

What is wrong with using CLF rather than CDF? If the former is possible, I would prefer to use it.

---

New commits:

d025f0f

Added a `__float__` method to the class Universal Cyclotomic Field

[sagetrac-vripoll]

New commits:

d025f0f

Added a `__float__` method to the class Universal Cyclotomic Field

[sagetrac-vripoll]

comment:4

I don't know, but what is better with CLF? A timing test is favorable towards CDF:

    sage: UCF.<E> = UniversalCyclotomicField()
    sage: b=E(5)+E(5)^(-1)
    sage: %timeit float(CDF(b).real_part())
    1000 loops, best of 3: 1.06 ms per loop
    sage: %timeit float(CLF(b))
    100 loops, best of 3: 3.41 ms per loop

[sagetrac-vripoll]

Author: Vivien Ripoll

[stumpc5]

comment:6

Replying to @sagetrac-vripoll:

I don't know, but what is better with CLF?

The ComplexLazyField gives you as much precision as you want, while the ComplexDoubleField is restricted

sage: a = CLF(e)
sage: b = CDF(e)
sage: a
2.718281828459046?
sage: b
2.71828182846

[mezzarobba]

comment:7

Replying to @stumpc5:

The ComplexLazyField gives you as much precision as you want, while the ComplexDoubleField is restricted

But __float__ is supposed to return a Python float, so you don't need the extra precision...

Vivien: wouldn't a ValueError be more appropriate in the case where the element you want to convert is not real?

[fchapoton]

Reviewer: Frédéric Chapoton

[fchapoton]

Changed commit from d025f0f to e5e01d0

[fchapoton]

comment:8

I have corrected the doctest, that was failing. And also changed to ValueError

And also used python3 syntax

This looks good to me. If nobody else disagrees, and my changes are ok, you can set to positive review.

---

New commits:

c71e5d9	trac #16120 correct doctest and python3 syntax
e5e01d0	trac #16120 change to ValueError

[fchapoton]

Changed branch from u/vripoll/ticket16120 to public/ticket/16120

[sagetrac-tmonteil]

comment:10

I was working on this review but you are all too fast for me :)

I have some issues with testing this method.

sage: UCF.<E> = UniversalCyclotomicField()
sage: e = 3/2*E(7) + E(7)^3 + E(7)^4 + 3/2*E(7)^6
sage: e.is_real()
True
sage: CDF(e)
0.0685316697714 + 1.62630325873e-19*I

Meaning that somewhere the rounding is not correct (at least for the imaginary
part) with the computation of CDF(e). Hence float(CDF(self).real_part())
looks like a post-processing of an error somewhere. But i do not see any
reason why the rounding of the real part will be better.

As your number is already real, i would prefer to do float(AA(e)): here the
"real part projection" is done on the exact side (and the method real_exact
of AA will be called only once). It is however between two and three times
slower.

What do you think about this ?

By the way, it could be nice to be able to have RR(e), RIF(e) and CIF(e)
work as well.

That said:

sage: UCF.<E> = UniversalCyclotomicField()
sage: e = 3/2*E(7) + E(7)^3 + E(7)^4 + 3/2*E(7)^6
sage: float(e) == float(AA(e))
False
sage: float(e) < float(AA(e))
True
sage: RIF = RealIntervalField(100)
sage: RIF(float(e)).endpoints()[0] < RIF(AA(e)).endpoints()[0] < RIF(AA(e)).endpoints()[1] < RIF(float(AA(e))).endpoints()[0]
True
sage: RIF(AA(e)).endpoints()[1] - RIF(float(e)).endpoints()[0]
5.9704982077798935821915405952e-19
sage: RIF(float(AA(e))).endpoints()[0] - RIF(AA(e)).endpoints()[1]
1.3280737987036467397076241791e-17

Meaning that e is closer to float(e) than float(AA(e)) which are both
floating point numbers of the same precision, and the default rounding mode is
done "to the nearest". The conversion from AA to RR (hence to float) is
assuming to respect the rounding mode, since the doc explicitely claims to
respect the rounding mode. This has to be explored, though it is not part of
this ticket.

[sagetrac-tmonteil]

Changed reviewer from Frédéric Chapoton to Frédéric Chapoton, Thierry Monteil

[mezzarobba]

comment:11

Replying to @sagetrac-tmonteil:

As your number is already real, i would prefer to do float(AA(e)): here the
"real part projection" is done on the exact side (and the method real_exact
of AA will be called only once). It is however between two and three times
slower.

What do you think about this ?

I prefer the current version, because, as far as I understand, the test that e is real implied by AA(e) (AA._element_constructor_ testing that the imaginary part of QQbar(e) is zero) can be much more expensive than e.is_real(). And I don't really see the point of computing an exact representation of the real part if what we want in the end is an approximation.eans that

By the way, it could be nice to be able to have RR(e), RIF(e) and CIF(e)
work as well.

Sure. But not necessarily as part of this ticket.

The conversion from AA to RR (hence to float) is
assuming to respect the rounding mode, since the doc explicitely claims to
respect the rounding mode.

From AA to RR, yes, but why "hence to float"?

That being said, I agree that the conversion from AA to RR does not seem to round to nearest:

sage: a = AA(e); r = RR(a)
sage: r.exact_rational() - a
1.328?e-17
sage: r.nextbelow().exact_rational() - a
-6.0?e-19

[mezzarobba]

comment:12

Replying to @sagetrac-tmonteil:

The conversion from AA to RR (hence to float) is
assuming to respect the rounding mode, since the doc explicitely claims to
respect the rounding mode.

Hmm, are you referring to the docstring of AlgebraicReal.real_number()? It states that "The approximation will be off by at most two ulp's" and "the rounding mode of the field is respected"... but what can it possibly mean to respect RNDN without guaranteeing an error < 1/2 ulp???

[sagetrac-vripoll]

comment:13

Thank you Fred for your review and correction of syntax. Thanks Marc and Thierry for the technical discussion, I'm learning a lot! I'll wait for some consensus before doing any change.

[fchapoton]

comment:14

So, what remains to be done here ?

[mezzarobba]

comment:15

Replying to @fchapoton:

So, what remains to be done here ?

I don't know: I am happy with the patch as it is, but Thierry seemed to disagree, and I'd like to hear his opinion on my last few comments...

[sagetrac-tmonteil]

comment:16

Replying to [comment:11]:

I prefer the current version, because, as far as I understand, the test that e is real implied by AA(e) (AA._element_constructor_ testing that the imaginary part of QQbar(e) is zero) can be much more expensive than e.is_real(). And I don't really see the point of computing an exact representation of the real part if what we want in the end is an approximation.

You are right about the timings (the AA method is between 2 and 3 times slower). But the advantage with float(AA(e)) is that you have a direct conversion from AA to RR, not a conversion inherited from the coercion mechanism, which is less easy to control (unless all parts of Sage are floating-point accurate, which is unfortunately not true).

By the way, it could be nice to be able to have RR(e), RIF(e) and CIF(e)
work as well.

Sure. But not necessarily as part of this ticket.

Of course, this was only a suggestion for a further work on universal cyclotomic field.

The conversion from AA to RR (hence to float) is
assuming to respect the rounding mode, since the doc explicitely claims to
respect the rounding mode.

From AA to RR, yes, but why "hence to float"?

Because the method __float__ of AA is just return float(RR(self)).

Replying to [comment:12]:

Hmm, are you referring to the docstring of AlgebraicReal.real_number()? It states that "The approximation will be off by at most two ulp's" and "the rounding mode of the field is respected"... but what can it possibly mean to respect RNDN without guaranteeing an error < 1/2 ulp???

Yes, this is meaningless and i plan to fix it soon, see #16163. It should not cost much more time than now to have the right rounding since the current method already uses MPFI, it is only a matter of used digits (the current choice could have made sense if it was using CPU 53 bits precision arithmetics).

Replying to [comment:15]:

I don't know: I am happy with the patch as it is, but Thierry seemed to disagree, and I'd like to hear his opinion on my last few comments...

If you are all ok with the current implementation, i am fine with it, especially i understood that this method is created for plotting. I will make stronger tests, if necessary i will open a new ticket, but i do not want to block this.

[sagetrac-tmonteil]

Changed reviewer from Frédéric Chapoton, Thierry Monteil to Frédéric Chapoton, Marc Mezzarobba, Thierry Monteil

[vbraun]

Changed branch from public/ticket/16120 to e5e01d0