Remove unnecessary signal handling for low prec mpfr operations. #7879

robertwb · 2010-01-09T19:31:01Z

_sig_on and _sig_off are more expensive than computing the result itself.

Apply only attachment: trac_7879-RR-signal.2.rebased_to_4.7.2.alpha2.patch to the Sage library.

Component: basic arithmetic

Keywords: sd32

Author: Robert Bradshaw

Reviewer: Alex Ghitza, Mariah Lenox, William Stein

Merged: sage-4.7.2.alpha3

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

The text was updated successfully, but these errors were encountered:

robertwb · 2010-01-09T19:38:47Z

comment:1

Signal handling is omitted for functions/precisions that were clearly and uniformly in the millisecond range or less.

robertwb · 2010-01-09T19:42:17Z

Attachment: 7879-RR-signal.patch.gz

aghitza · 2010-01-20T10:24:37Z

Reviewer: Alex Ghitza

aghitza · 2010-01-20T10:24:37Z

Author: Robert Bradshaw

aghitza · 2010-01-20T10:24:37Z

comment:3

This looks good. I have one question: you are hardcoding threshold precisions after which the signals should be used, mostly 1000, but also 10000 in a couple of places. Would it be better to have a global variable PREC_THRESHOLD set to 1000 at the top of the file, and then use it or 10*PREC_THRESHOLD where needed? It would make it easier to change this in the future, since a search for it would be all that's needed.

I'm marking this as needs_info. If you think it's not worth the trouble, I'll put a positive review.

jdemeyer · 2011-01-19T16:10:19Z

comment:4

I am proposing to close this ticket as invalid since #9678 makes sig_on() and sig_off() a lot faster. In any case, if you want to keep the patch, it needs rebasing (_sig_on and _sig_off must be replaced by sig_on() and sig_off() but there are more patch conflicts).

sagetrac-mariah · 2011-05-17T13:19:43Z

comment:5

Since this ticket is marked sage-duplicate/invalid/wontfix plus has
not been active for 4 months, I believe this ticket can be closed.

jdemeyer · 2011-05-17T15:56:22Z

comment:6

Robert, what is your opinion? sig_on() and sig_off() became a lot faster since #9678, but they still take up some cycles.

robertwb · 2011-05-17T18:26:12Z

comment:7

Thanks for taking a look at this. I've been waiting for 4.7 to be released to re-base this, and think it'd still worth having. Perhaps I'll put PREC_THRESHOLD in there while I'm at it, and maybe do some benchmarking.

jdemeyer · 2011-05-17T20:33:12Z

comment:9

Replying to @robertwb:

and maybe do some benchmarking.

If you find useful ways to do benchmarks, please let me know. I might re-use those benchmarks to optimize sig_on() and sig_off().

jdemeyer · 2011-05-17T20:34:06Z

comment:10

The patch adds one unrelated doctest. This doctest has been moved to #11344.

robertwb · 2011-05-31T08:01:09Z

comment:11

----------------------------------------------------------------------
| Sage Version 4.7, Release Date: 2011-05-23                         |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
Loading Sage library. Current Mercurial branch is: bitrot

sage: L = [RR(k) for k in range(1000)]

sage: %timeit [a.sqrt() for a in L]
 625 loops, best of 3: 569 µs per loop

sage: %timeit [a.sqrt() for a in L]
 625 loops, best of 3: 509 µs per loop

sage: %timeit [a.sqrt() for a in L]
 625 loops, best of 3: 513 µs per loop

sage: timeit("[a.sqrt() for a in L]", number=10000)
10000 loops, best of 3: 519 µs per loop

sage: 
Exiting Sage (CPU time 0m19.94s, Wall time 1m9.75s).
robertwb@sage:/scratch/robertwb/sage-4.7/devel/sage-bitrot$ hg qpush
applying 7879-RR-signal.patch
now at: 7879-RR-signal.patch
robertwb@sage:/scratch/robertwb/sage-4.7/devel/sage-bitrot$ ../../sage -br
[...]
----------------------------------------------------------------------
| Sage Version 4.7, Release Date: 2011-05-23                         |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
Loading Sage library. Current Mercurial branch is: bitrot

sage: L = [RR(k) for k in range(1000)]

sage: %timeit [a.sqrt() for a in L]
 625 loops, best of 3: 498 µs per loop

sage: %timeit [a.sqrt() for a in L]
 625 loops, best of 3: 498 µs per loop

sage: %timeit [a.sqrt() for a in L]
 625 loops, best of 3: 496 µs per loop

sage: timeit("[a.sqrt() for a in L]", number=10000)
10000 loops, best of 3: 499 µs per loop

Looks like about a 5% win (much smaller than I remember it being before).

robertwb · 2011-05-31T08:01:57Z

Attachment: 7879-RR-signal.2.patch.gz

Rebased on 4.7

sagetrac-mariah · 2011-06-01T15:19:06Z

comment:12

patch attachment: 7879-RR-signal.2.patch applied to sage-4.7.1.alpha1. Then did 'sage -b' followed by 'make testlong'.
The following test failed:

eno% ./sage -t  -long -force_lib "devel/sage/sage/rings/residue_field.pyx"
sage -t -long -force_lib "devel/sage/sage/rings/residue_field.pyx"
**********************************************************************
File "/home/mariah/sage/sage-4.7.1.alpha1-x86_64-Linux-core2-fc-review-7879/devel/sage/sage/rings/residue_field.pyx", line 670:
    sage: I = QQ[3^(1/3)].factor(5)[1][0]; I
Exception raised:
    Traceback (most recent call last):
      File "/home/mariah/sage/sage-4.7.1.alpha1-x86_64-Linux-core2-fc-review-7879/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/home/mariah/sage/sage-4.7.1.alpha1-x86_64-Linux-core2-fc-review-7879/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/home/mariah/sage/sage-4.7.1.alpha1-x86_64-Linux-core2-fc-review-7879/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_10[2]>", line 1, in <module>
        I = QQ[Integer(3)**(Integer(1)/Integer(3))].factor(Integer(5))[Integer(1)][Integer(0)]; I###line 670:
    sage: I = QQ[3^(1/3)].factor(5)[1][0]; I
      File "ring.pyx", line 167, in sage.rings.ring.Ring.__getitem__ (sage/rings/ring.c:2007)
      File "expression.pyx", line 8287, in sage.symbolic.expression.Expression.minpoly (sage/symbolic/expression.cpp:31950)
      File "/home/mariah/sage/sage-4.7.1.alpha1-x86_64-Linux-core2-fc-review-7879/local/lib/python/site-packages/sage/calculus/calculus.py", line 925, in minpoly
        if g(ex).simplify_trig().simplify_radical() == 0:
      File "expression.pyx", line 6546, in sage.symbolic.expression.Expression.simplify_trig (sage/symbolic/expression.cpp:24657)
      File "expression.pyx", line 460, in sage.symbolic.expression.Expression._maxima_ (sage/symbolic/expression.cpp:3674)
      File "sage_object.pyx", line 429, in sage.structure.sage_object.SageObject._interface_ (sage/structure/sage_object.c:3451)
      File "lazy_import.pyx", line 167, in sage.misc.lazy_import.LazyImport.__getattr__ (sage/misc/lazy_import.c:1353)
      File "lazy_import.pyx", line 110, in sage.misc.lazy_import.LazyImport._get_object (sage/misc/lazy_import.c:1023)
      File "/home/mariah/sage/sage-4.7.1.alpha1-x86_64-Linux-core2-fc-review-7879/local/lib/python/site-packages/sage/interfaces/maxima_lib.py", line 152, in <module>
        ecl_eval("#$%s$"%l)
      File "ecl.pyx", line 1224, in sage.libs.ecl.ecl_eval (sage/libs/ecl.c:6301)
      File "ecl.pyx", line 1239, in sage.libs.ecl.ecl_eval (sage/libs/ecl.c:6252)
      File "ecl.pyx", line 252, in sage.libs.ecl.ecl_safe_eval (sage/libs/ecl.c:2544)
    RuntimeError: ECL says: THROW: The catch MACSYMA-QUIT is undefined.
**********************************************************************
File "/home/mariah/sage/sage-4.7.1.alpha1-x86_64-Linux-core2-fc-review-7879/devel/sage/sage/rings/residue_field.pyx", line 672:
    sage: k = I.residue_field(); k
Exception raised:
    Traceback (most recent call last):
      File "/home/mariah/sage/sage-4.7.1.alpha1-x86_64-Linux-core2-fc-review-7879/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/home/mariah/sage/sage-4.7.1.alpha1-x86_64-Linux-core2-fc-review-7879/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/home/mariah/sage/sage-4.7.1.alpha1-x86_64-Linux-core2-fc-review-7879/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_10[3]>", line 1, in <module>
        k = I.residue_field(); k###line 672:
    sage: k = I.residue_field(); k
      File "element.pyx", line 328, in sage.structure.element.Element.__getattr__ (sage/structure/element.c:2792)
      File "parent.pyx", line 277, in sage.structure.parent.getattr_from_other_class (sage/structure/parent.c:2940)
      File "parent.pyx", line 175, in sage.structure.parent.raise_attribute_error (sage/structure/parent.c:2709)
    AttributeError: 'sage.symbolic.expression.Expression' object has no attribute 'residue_field'
**********************************************************************

etc.

williamstein · 2011-08-23T07:16:55Z

comment:13

Replying to @sagetrac-mariah:

patch attachment: 7879-RR-signal.2.patch applied to sage-4.7.1.alpha1. Then did 'sage -b' followed by 'make testlong'.
The following test failed:

eno% ./sage -t  -long -force_lib "devel/sage/sage/rings/residue_field.pyx"
sage -t -long -force_lib "devel/sage/sage/rings/residue_field.pyx"
**********************************************************************
File "/home/mariah/sage/sage-4.7.1.alpha1-x86_64-Linux-core2-fc-review-7879/devel/sage/sage/rings/residue_field.pyx", line 670:
    sage: I = QQ[3^(1/3)].factor(5)[1][0]; I
Exception raised:

These errors don't occur for me with sage-4.7.2.alpha2.

williamstein · 2011-08-23T07:16:55Z

Changed reviewer from Alex Ghitza to Alex Ghitza, William Stein

williamstein · 2011-08-23T07:24:14Z

comment:14

I did a full test of Sage on another machine too with 4.7.2.alpha2, and everything works. Code looks good and addresses other people's issues...

williamstein · 2011-08-24T23:43:12Z

Changed keywords from none to sd32

nexttime · 2011-09-09T04:24:50Z

Changed reviewer from Alex Ghitza, William Stein to Alex Ghitza, Mariah Lenox, William Stein

nexttime · 2011-09-12T15:51:10Z

Sage library patch. Robert's patch rebased to Sage 4.7.2.alpha2, because of fuzz 2.

nexttime · 2011-09-12T15:52:24Z

Attachment: trac_7879-RR-signal.2.rebased_to_4.7.2.alpha2.patch.gz

Attachment: trac_7879-harmless_fuzz_2.diff.gz

Do not apply. Fuzz against Sage 4.7.2.alpha2. For reference only.

nexttime · 2011-09-12T15:58:22Z

comment:17

Attached a rebased patch because of fuzz.

nexttime · 2011-09-13T12:16:11Z

Merged: sage-4.7.2.alpha3

robertwb added this to the sage-4.6.2 milestone Jan 9, 2010

robertwb added c: basic arithmetic labels Jan 9, 2010

robertwb assigned aghitza Jan 9, 2010

robertwb added the s: needs review label Jan 9, 2010

This comment has been minimized.

Sign in to view

aghitza added s: needs info and removed s: needs review labels Jan 20, 2010

jdemeyer removed this from the sage-4.6.2 milestone Jan 19, 2011

jdemeyer added s: needs review and removed s: needs info labels Jan 19, 2011

sagetrac-mariah mannequin added s: positive review and removed s: needs review labels May 17, 2011

robertwb added this to the sage-4.7.1 milestone May 17, 2011

sagetrac-mariah mannequin added s: needs work and removed s: positive review labels May 17, 2011

robertwb removed the s: needs work label May 31, 2011

robertwb added the s: needs review label May 31, 2011

sagetrac-mariah mannequin added s: needs work and removed s: needs review labels Jun 1, 2011

williamstein added s: positive review and removed s: needs work labels Aug 23, 2011

This comment has been minimized.

Sign in to view

nexttime mannequin removed the s: positive review label Sep 13, 2011

nexttime mannequin closed this as completed Sep 13, 2011

This was referenced Jul 13, 2015

Rewrite interrupt handling #9678

Closed

Add log1p() example #11344

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove unnecessary signal handling for low prec mpfr operations. #7879

Remove unnecessary signal handling for low prec mpfr operations. #7879

robertwb commented Jan 9, 2010

robertwb commented Jan 9, 2010

This comment has been minimized.

robertwb commented Jan 9, 2010

aghitza commented Jan 20, 2010

aghitza commented Jan 20, 2010

aghitza commented Jan 20, 2010

jdemeyer commented Jan 19, 2011

sagetrac-mariah mannequin commented May 17, 2011

jdemeyer commented May 17, 2011

robertwb commented May 17, 2011

jdemeyer commented May 17, 2011

jdemeyer commented May 17, 2011

robertwb commented May 31, 2011

robertwb commented May 31, 2011

sagetrac-mariah mannequin commented Jun 1, 2011

williamstein commented Aug 23, 2011

williamstein commented Aug 23, 2011

williamstein commented Aug 23, 2011

williamstein commented Aug 24, 2011

This comment has been minimized.

nexttime mannequin commented Sep 9, 2011

nexttime mannequin commented Sep 12, 2011

nexttime mannequin commented Sep 12, 2011

This comment has been minimized.

nexttime mannequin commented Sep 12, 2011

nexttime mannequin commented Sep 13, 2011

Remove unnecessary signal handling for low prec mpfr operations. #7879

Remove unnecessary signal handling for low prec mpfr operations. #7879

Comments

robertwb commented Jan 9, 2010

robertwb commented Jan 9, 2010

This comment has been minimized.

robertwb commented Jan 9, 2010

aghitza commented Jan 20, 2010

aghitza commented Jan 20, 2010

aghitza commented Jan 20, 2010

jdemeyer commented Jan 19, 2011

sagetrac-mariah mannequin commented May 17, 2011

jdemeyer commented May 17, 2011

robertwb commented May 17, 2011

jdemeyer commented May 17, 2011

jdemeyer commented May 17, 2011

robertwb commented May 31, 2011

robertwb commented May 31, 2011

sagetrac-mariah mannequin commented Jun 1, 2011

williamstein commented Aug 23, 2011

williamstein commented Aug 23, 2011

williamstein commented Aug 23, 2011

williamstein commented Aug 24, 2011

This comment has been minimized.

nexttime mannequin commented Sep 9, 2011

nexttime mannequin commented Sep 12, 2011

nexttime mannequin commented Sep 12, 2011

This comment has been minimized.

nexttime mannequin commented Sep 12, 2011

nexttime mannequin commented Sep 13, 2011