Opened 11 years ago

Closed 11 years ago

#3762 closed enhancement (fixed)

[with patch, positive review] deprecate quaddouble, switch the number of partitions code to use MPFR all the way

Reported by: was Owned by: mabshoff
Priority: major Milestone: sage-3.3
Component: distribution Keywords:
Cc: cwitty Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Note -- also close #3079.

I wonder -- should quaddouble should be moved to be an optional spkg? That will be weird, since the actual support for quaddouble is all over the sage install (in all the coercions code, etc.).

Attachments (5)

3762-partitions-tests.patch (2.0 KB) - added by robertwb 11 years ago.
adds a test code wrapper for sage
3762-partitions-qd.patch (11.7 KB) - added by robertwb 11 years ago.
makes qd dependance of partitions optional
3762-remove-qd.patch (30.5 KB) - added by robertwb 11 years ago.
3762-qd-real-roots.patch (3.6 KB) - added by robertwb 11 years ago.
3762-qd-fixes.patch (1.5 KB) - added by robertwb 11 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 11 years ago by was

My real_roots.pyx uses a couple of functions from quaddouble:
fpu_fix_start and fpu_fix_end.  (partitions_c.cc also uses these
functions.)  These are found in the quaddouble source in src/fpu.cpp
and include/fpu.h

Probably the right fix is to copy only those two files into c_lib/.  I
can do that, but I'll need some help with configuration.  (The main
thing is that the files want a preprocessor symbol X86 to be defined
iff we're using an x86-family processor.)

Carl

comment:2 Changed 11 years ago by robertwb

  • Summary changed from remove quaddouble from sage -- not used, source of pain, mpfr is better to [with patch, not ready for review] remove quaddouble from sage -- not used, source of pain, mpfr is better

Changed 11 years ago by robertwb

adds a test code wrapper for sage

Changed 11 years ago by robertwb

makes qd dependance of partitions optional

comment:3 Changed 11 years ago by robertwb

I removed the quad-double dependancy from the partitions counting code, but there seems to be a significant speed regression :(.

Changed 11 years ago by robertwb

Changed 11 years ago by robertwb

comment:4 Changed 11 years ago by robertwb

  • Cc cwitty added
  • Summary changed from [with patch, not ready for review] remove quaddouble from sage -- not used, source of pain, mpfr is better to [with patch, needs review] remove quaddouble from sage -- not used, source of pain, mpfr is better

This removes all sage dependancies on quaddouble, but leaves it in to be deprecated for a while.

Some issues that need to be dealt with are (1) the speed regression in partitions and (2) whether the fix for real_roots would better be extracting fpu_fix from the qd autoconf script. Perhaps if qd is present, we could detect it and compile the above with the qd routines.

comment:5 Changed 11 years ago by was

  • Milestone changed from sage-3.4.1 to sage-3.3
  • Summary changed from [with patch, needs review] remove quaddouble from sage -- not used, source of pain, mpfr is better to [with patch, positive review] remove quaddouble from sage -- not used, source of pain, mpfr is better
  1. I applied this to sage-3.3.alpha1 on sage.math and got:
    The following tests failed:
    
            sage -t  devel/sage/sage/rings/complex_field.py # 1 doctests failed
            sage -t  devel/sage/sage/structure/sage_object.pyx # 1 doctests failed
    ----------------------------------------------------------------------
    Total time for all tests: 164.0 seconds
    wstein@sage:/scratch/wstein/sage-3.3.alpha1$  
    

We have:

sage -t  devel/sage/sage/structure/sage_object.pyx**********************************************************************
File "/scratch/wstein/sage-3.3.alpha1/devel/sage-main/sage/structure/sage_object.pyx", line 682:
    sage: sage.structure.sage_object.unpickle_all(std)
Expected:
    doctest:...: DeprecationWarning: Your data is stored in an old format. Please use the save() function to store your data i
n a more recent format.
    Successfully unpickled ... objects.
    Failed to unpickle 0 objects.
Got:
    doctest:1172: DeprecationWarning: Your data is stored in an old format. Please use the save() function to store your data 
in a more recent format.
    doctest:1172: DeprecationWarning: RQDF is deprecated; use RealField(212) instead.
    Successfully unpickled 448 objects.
    Failed to unpickle 0 objects.
**********************************************************************
1 items had failures:
   1 of   7 in __main__.example_16
***Test Failed*** 1 failures.

sage -t  devel/sage/sage/rings/complex_field.py
**********************************************************************
File "/scratch/wstein/sage-3.3.alpha1/devel/sage-main/sage/rings/complex_field.py", line 105:
    sage: C(RR.log2(), RR.e())
Exception raised:
    Traceback (most recent call last):
      File "/scratch/mabshoff/sage-3.3.alpha1/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/scratch/mabshoff/sage-3.3.alpha1/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/scratch/mabshoff/sage-3.3.alpha1/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_2[7]>", line 1, in <module>
        C(RR.log2(), RR.e())###line 105:
    sage: C(RR.log2(), RR.e())
    AttributeError: 'sage.rings.real_mpfr.RealField' object has no attribute 'e'
**********************************************************************
1 items had failures:

  1. Performance on sage.math:
    BEFORE:
    sage: time k = number_of_partitions(10^8)
    CPU times: user 2.48 s, sys: 0.00 s, total: 2.48 s
    Wall time: 2.49 s
    sage: time k = number_of_partitions(10^9)
    CPU times: user 22.85 s, sys: 0.00 s, total: 22.85 s
    Wall time: 22.85 s
    
    
    AFTER:
    sage: time k = number_of_partitions(10^8)
    CPU times: user 3.29 s, sys: 0.00 s, total: 3.29 s
    Wall time: 3.29 s
    sage: time k = number_of_partitions(10^9)
    CPU times: user 35.68 s, sys: 0.00 s, total: 35.68 s
    Wall time: 35.71 s
    
  1. I read all the code in the patches, and am fine with it.

Positive review if the two doctest issues above are fixed.

NOTE: you should not delete the quaddouble package, since this code still links it in. That will have to happen in a few months after Deprecation.

Changed 11 years ago by robertwb

comment:6 Changed 11 years ago by robertwb

Attach all 5 patches in order.

Followup for speed regression at #5084 and fpu_fix at #5085.

comment:7 Changed 11 years ago by mabshoff

  • Summary changed from [with patch, positive review] remove quaddouble from sage -- not used, source of pain, mpfr is better to [with patch, positive review] deprecate quaddouble, switch the number of partitions code to use MPFR all the way

comment:8 Changed 11 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from new to closed

Merged all five patches in Sage 3.3.alpha3.

Cheers,

Michael

Note: See TracTickets for help on using tickets.