search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.38k stars 471 forks source link

modernize complex_mpfr #24489

Open videlec opened 6 years ago

videlec commented 6 years ago

Similarly to #24457 for real numbers we perform some cleaning for complex numbers in view of #17713/#24457.

step 1

see also task ticket #17713

Depends on #24483 Depends on #24457

CC: @mezzarobba @jpflori

Component: basic arithmetic

Author: Vincent Delecroix

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

videlec commented 6 years ago

Description changed:

--- 
+++ 
@@ -2,4 +2,5 @@
 - rename `CompleNumber`/`ComplexField` into `ComplexFloatingPoint`/`ComplexFloatingPointField`
 - remove the attribute `_prec` of `ComplexNumber` (a `mpfr_t` carries its precision that can be obtained with `mpfr_get_prec`)
 - deprecate `is_ComplexNumber(x)`/`is_ComplexField(x)` in favor of `isinstance(x, ComplexFloatingPoint)`/`isinstance(x, ComplexFloatingPointField)`
+- actually initialize the `mpfr_t` pointers in `__cinit__` as it is the case for real floating point numbers in `real_mpfr.pyx`
 - clarify the behavior of rounding (currently there is a global (sic) variable taking care of it)
videlec commented 6 years ago

Description changed:

--- 
+++ 
@@ -4,3 +4,4 @@
 - deprecate `is_ComplexNumber(x)`/`is_ComplexField(x)` in favor of `isinstance(x, ComplexFloatingPoint)`/`isinstance(x, ComplexFloatingPointField)`
 - actually initialize the `mpfr_t` pointers in `__cinit__` as it is the case for real floating point numbers in `real_mpfr.pyx`
 - clarify the behavior of rounding (currently there is a global (sic) variable taking care of it)
+- change the string representation from `Complex Field with XX bits of precision` to `Complex Floating-point Field with XX bits of precision`
videlec commented 6 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,5 @@
+Similarly to #24456 for real numbers we perform some cleaning for complex numbers
+
 - get rid of the factory `ComplexField` by making the class `ComplexField_class` inherits from `UniqueRepresentation`
 - rename `CompleNumber`/`ComplexField` into `ComplexFloatingPoint`/`ComplexFloatingPointField`
 - remove the attribute `_prec` of `ComplexNumber` (a `mpfr_t` carries its precision that can be obtained with `mpfr_get_prec`)
@@ -5,3 +7,4 @@
 - actually initialize the `mpfr_t` pointers in `__cinit__` as it is the case for real floating point numbers in `real_mpfr.pyx`
 - clarify the behavior of rounding (currently there is a global (sic) variable taking care of it)
 - change the string representation from `Complex Field with XX bits of precision` to `Complex Floating-point Field with XX bits of precision`
+- Deprecate `CC` in favor of `CFF`
jdemeyer commented 6 years ago
comment:4

If you are going to do serious refactoring, here is a different proposal: deprecate complex_mpfr altogether and use complex_mpc instead as the default complex floating point field.

videlec commented 6 years ago
comment:5

Replying to @jdemeyer:

If you are going to do serious refactoring, here is a different proposal: deprecate complex_mpfr altogether and use complex_mpc instead as the default complex floating point field.

+1. I wanted to do that at some point but Marc Mezzarobba claimed that the mpfr version was faster and hence still needed. I will be more than happy to recycle this ticket in order to do this!

Though the branch in #24483 is still useful to liberate the module sage.rings.complex_field needed for #24456.

jdemeyer commented 6 years ago
comment:6

+1. I wanted to do that at some point but Marc Mezzarobba claimed that the mpfr version was faster and hence still needed.

Please keep in mind #24353 which will almost certainly change timings. Unfortunately, that ticket is current stalled because it breaks MPFI. If there is a proper release of MPC, maybe I'll try to patch MPFI in Sage.

videlec commented 6 years ago

Description changed:

--- 
+++ 
@@ -1,5 +1,6 @@
 Similarly to #24456 for real numbers we perform some cleaning for complex numbers

+- deprecate import of `ComplexField` from `sage.rings.complex_field` (leftover from #24483)
 - get rid of the factory `ComplexField` by making the class `ComplexField_class` inherits from `UniqueRepresentation`
 - rename `CompleNumber`/`ComplexField` into `ComplexFloatingPoint`/`ComplexFloatingPointField`
 - remove the attribute `_prec` of `ComplexNumber` (a `mpfr_t` carries its precision that can be obtained with `mpfr_get_prec`)
videlec commented 6 years ago

Description changed:

--- 
+++ 
@@ -1,4 +1,4 @@
-Similarly to #24456 for real numbers we perform some cleaning for complex numbers
+Similarly to #24457 for real numbers we perform some cleaning for complex numbers

 - deprecate import of `ComplexField` from `sage.rings.complex_field` (leftover from #24483)
 - get rid of the factory `ComplexField` by making the class `ComplexField_class` inherits from `UniqueRepresentation`
@@ -9,3 +9,5 @@
 - clarify the behavior of rounding (currently there is a global (sic) variable taking care of it)
 - change the string representation from `Complex Field with XX bits of precision` to `Complex Floating-point Field with XX bits of precision`
 - Deprecate `CC` in favor of `CFF`
+
+see also task ticket #17713
videlec commented 6 years ago

Description changed:

--- 
+++ 
@@ -1,13 +1,15 @@
-Similarly to #24457 for real numbers we perform some cleaning for complex numbers
+Similarly to #24457 for real numbers we perform some cleaning for complex numbers in view of #17713/#24457.

-- deprecate import of `ComplexField` from `sage.rings.complex_field` (leftover from #24483)
-- get rid of the factory `ComplexField` by making the class `ComplexField_class` inherits from `UniqueRepresentation`
+## step 1
+
+- #24483: move `sage.rings.complex_field` to `sage.rings.complex_mpfr`
+- change the string representation from `Complex Field with XX bits of precision` to `Complex Floating-point Field with XX bits of precision`
+- (possibly) get rid of the factory `ComplexField` by making the class `ComplexField_class` inherits from `UniqueRepresentation`
 - rename `CompleNumber`/`ComplexField` into `ComplexFloatingPoint`/`ComplexFloatingPointField`
 - remove the attribute `_prec` of `ComplexNumber` (a `mpfr_t` carries its precision that can be obtained with `mpfr_get_prec`)
 - deprecate `is_ComplexNumber(x)`/`is_ComplexField(x)` in favor of `isinstance(x, ComplexFloatingPoint)`/`isinstance(x, ComplexFloatingPointField)`
 - actually initialize the `mpfr_t` pointers in `__cinit__` as it is the case for real floating point numbers in `real_mpfr.pyx`
 - clarify the behavior of rounding (currently there is a global (sic) variable taking care of it)
-- change the string representation from `Complex Field with XX bits of precision` to `Complex Floating-point Field with XX bits of precision`
 - Deprecate `CC` in favor of `CFF`

 see also task ticket #17713
videlec commented 6 years ago

Changed dependencies from #24483 to #24483, #24457

videlec commented 6 years ago
comment:10

Replying to @videlec:

Replying to @jdemeyer:

+1. I wanted to do that at some point but Marc Mezzarobba claimed that the mpfr version was faster and hence still needed. I will be more than happy to recycle this ticket in order to do this!

My bad: it was JP Flori.

jpflori commented 6 years ago
comment:12

Yes it used to be the case, and Paul Zimmerman improved MPC but my last souvenir is that for basic operations Sage's complex_mpfr was still faster than complex_mpc surely because it does not handle special cases (NaN, infinities, and i don't know what) gracefully.

Things can have changed but there is only one way to knwom: benchmark both implementations, and I don't think I have any time for this.

On a side note, I would think it is a very good idea to get rid of complex_mpfr if we can.

jdemeyer commented 6 years ago
comment:13

Replying to @jpflori:

because it does not handle special cases (NaN, infinities, and i don't know what) gracefully.

Certainly not because of that reason. First of all, checking for a special value is really trivial compared to dealing with Python objects. You need to work with least ~100 bits of precision to have a sensible benchmark because otherwise you are only benchmarking the Python overhead anyway.

jdemeyer commented 6 years ago
comment:14

Replying to @jpflori:

my last souvenir is that for basic operations Sage's complex_mpfr was still faster than complex_mpc

Of course it's always going to be faster. But that's not the point. If you really want speed, use CDF.

The thing that we should focus on is the correctness. With MPC, you are guaranteed that the answer that you receive is as good as it can be. With MPFR complex numbers, we are using some arbitrary formulas and we hope that everything works. On the one hand, we use an arbitrary-precision library but we cannot say whether the many bits that you get are actually meaningful.