msg77223 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2008-12-07 12:42 |
In issue 4506, Skip reported that test_cmath fails on Solaris 10/x86 for
3.0. If my guesses are correct, it probably fails on all x86 systems that
(a) use the x87 coprocessor for floating-point (as opposed to using SSE2,
for example), and (b) don't have isinf available.
Problem: Py_IS_INFINITY is applied to a floating-point value sitting in an
80-bit x87 register; that value is not infinity, but after moving it back
to memory (and hence rounding from 80-bit extended precision to 64-bit
double precision with its smaller exponent range) it becomes infinity.
Solution: Add a macro to pymath.h that forces rounding from extended
precision to double precision; apply this macro *before* calling
Py_IS_INFINITY. See attached patch for an example.
Problem: After applying the attached patch to the py3k branch, the cmath
module fails to build. On OS X 10.5, I get:
running build_ext
building 'cmath' extension
gcc -bundle -undefined dynamic_lookup build/temp.macosx-10.3-i386-
3.1/Users/dickinsm/python_source/py3k/Modules/cmathmodule.o -
L/usr/local/lib -o build/lib.macosx-10.3-i386-3.1/cmath.so
*** WARNING: renaming "cmath" since importing it failed:
dlopen(build/lib.macosx-10.3-i386-3.1/cmath.so, 2): Symbol not found:
_Py_force_to_memory
Referenced from: /Users/dickinsm/python_source/py3k/build/lib.macosx-
10.3-i386-3.1/cmath.so
Expected in: dynamic lookup
Solution: ???
Christian, as the architect of pymath.h, do you have any ideas what I'm
doing wrong? Or comments on the patch in general? What do I need to do
to make Py_force_to_memory visible to extension modules?
|
msg77811 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2008-12-14 18:05 |
Here's a patch (force_to_memory2.patch) that I'm hoping fixes the cmath
test failures on Solaris 10/x86.
Skip, could you give it a try?
The patch isn't final: I need to look for more places where
Py_FORCE_DOUBLE should be applied. But I'll wait to find out whether this
really does fix the problem, first; I'm still just guessing about the
cause.
By the way, it looks like the problem with the original patch, on OS X,
was that nothing in pymath.c is used in the Python *core*, so the symbols
from pymath.o aren't compiled into the python.exe executable, and they're
not available when loading modules. Rather than working out how to fix
this, I just moved the definitions into Objects/floatobject.c instead.
|
msg77822 - (view) |
Author: Roumen Petrov (rpetrov) * |
Date: 2008-12-14 20:41 |
may be proposed patch break platforms that need specific "export" decoration
|
msg77826 - (view) |
Author: Skip Montanaro (skip.montanaro) * |
Date: 2008-12-14 21:08 |
Mark> Skip, could you give it a try?
Works for me on Solaris 10/x86. Based on Roumen's comment I am preparing to
try it on Mac OS X/x86 and Solaris 10/sparc.
Skip
|
msg77830 - (view) |
Author: Roumen Petrov (rpetrov) * |
Date: 2008-12-14 21:22 |
I'm not sure that patch has to deal with "force to memory".
FYI: when I port python trunk to mingw platform I encounter some
inconsistency for isinf.
My note about issue follow:
- configure.in:
...
dnl FIXME: For mingw "isinf" is a macro defined in <math.h> and can't be
dnl detected as function. Also PC/pyconfig.h define HAVE_ISINF but it is
dnl useless since header define Py_IS_INFINITY as (!_finite(X) &&
!_isnan(X))
dnl Also in pymath.h is commented too how PC/pyconfig.h define _isinf.
dnl NOTE: For mingw to keep compatibility with native build we define
dnl Py_IS_INFINITY in pyport.h.
...
- pyport.h
#ifdef __MINGW32__
...
/*NOTE: mingw has isinf as macro defined in math.h.
Since PC/pyconfig.h define Py_IS_INFINITY(X) that cover HAVE_ISINF
here for Py_IS_INFINITY we define same as for native build.
This makes HAVE_ISINF needless.
Also see comments in configure.in and pymath.h. */
#define Py_IS_INFINITY(X) (!_finite(X) && !_isnan(X))
....
next lest see Py_IS_INFINITY in pymath.h
....
#ifndef Py_IS_INFINITY
#ifdef HAVE_ISINF
#define Py_IS_INFINITY(X) isinf(X)
#else
#define Py_IS_INFINITY(X) ((X) && (X)*0.5 == (X))
#endif
#endif
....
Is the macro Py_IS_INFINITY correct if isinf is not detected by
configure script ?
Also I think too that problem is that we has to classify result after
conversion to double.
|
msg77832 - (view) |
Author: Roumen Petrov (rpetrov) * |
Date: 2008-12-14 21:26 |
As Skip Montanaro report that work on ... may be macro from pymath.h has
to be changed to force conversion to double.
|
msg77834 - (view) |
Author: Roumen Petrov (rpetrov) * |
Date: 2008-12-14 21:30 |
about "export" decoration it is find in then patch PyAPI_FUNC(double) .
found it after second review
|
msg77837 - (view) |
Author: Roumen Petrov (rpetrov) * |
Date: 2008-12-14 21:47 |
The macro Py_IS_INFINITY don't work on linux. The test case(force-inf.c)
is attached. The result don't depend from optimisation flag.
isinf(x)=1
Py_IS_INFINITY(x)=0
Py_IS_INFINITY2(x)=1
isinf(x)=0
Py_IS_INFINITY(x)=0
Py_IS_INFINITY2(x)=0
|
msg77841 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2008-12-14 22:03 |
> The macro Py_IS_INFINITY don't work on linux. The test case(force-inf.c)
> is attached. The result don't depend from optimisation flag.
Thanks, Roumen. I rather suspected that Py_IS_INFINITY was dodgy this
way.
On the other hand, this is only a problem when Py_IS_INFINITY is applied
to a *computed* result; most of the time Py_IS_INFINITY is being directly
applied to the incoming argument to some function, so should be safe.
|
msg77876 - (view) |
Author: Skip Montanaro (skip.montanaro) * |
Date: 2008-12-15 16:55 |
Took me awhile to locate a SPARC C compiler on our dwindling set of
Solaris/SPARC boxes at work, but I eventually found one and got Subversion
trunk to compile. test_cmath and test_math both pass with the
force_to_memory2 patch. I don't know if I mentioned it earlier (replying by
email), but it also works for me on Mac OS X 10.5.5 (Intel Core2 Duo).
Skip
|
msg77878 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2008-12-15 17:09 |
Thanks, Skip. Looks like this problem is 'solved in principle'. Now I
have to figure out a non-hackish solution.
|
msg78231 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2008-12-23 12:54 |
Final patch.
Skip, could you please give this one a try, and then with any luck this
can be fixed for 3.0.1. (Sorry for creating more work; this should be
the last time.)
I've added a configure test that detects x87 FPU usage, via the double
rounding issue associated with the x87. If x87 is detected then
_Py_force_double is used via a macro Py_FORCE_DOUBLE; otherwise,
Py_FORCE_DOUBLE does nothing.
When you configure on a machine that uses x87, you should see:
checking for x87-style double rounding... yes
in the configure output.
|
msg79088 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-01-04 22:27 |
Looking at this again, I don't like my solution.
I think it would be better to fix Py_IS_INFINITY directly, putting all the
complication into one place; then users of Py_IS_INFINITY don't have to
spend time worrying about whether they should be calling Py_FORCE_DOUBLE
or not.
The fixed Py_IS_INFINITY will likely be slower, but this only matters on
platforms that don't provide isinf, isnan; it seems that Solaris is the
only such platform in common use.
|
msg79091 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-01-04 22:57 |
Tim, I'm in need of some advice on Py_IS_INFINITY. It's currently
implemented (on platforms that don't provide isinf) as
#define Py_IS_INFINITY(X) ((X) && (X)*0.5 == (X))
I'd like to rewrite it as something like:
#define Py_IS_INFINITY_D(X) ((X) < -DBL_MAX || (X) > DBL_MAX)
#define Py_IS_INFINITY_F(X) ((X) < -FLT_MAX || (X) > FLT_MAX)
#define Py_IS_INFINITY(X) (sizeof(X) == sizeof(double) ? Py_IS_INFINITY_D(X) : Py_IS_INFINITY_F(X))
Are there any hidden (or obvious) numerical pitfalls with
this approach?
The reason for the rewrite is that the current Py_IS_INFINITY
can give false positives on x86 for values that are
pretending to be doubles, but are actually coming from an
80-bit x87 register.
|
msg79092 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-01-04 23:01 |
s/false positives/false negatives/
|
msg79093 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-01-04 23:14 |
Answering my own question, there *are* pitfalls: (X) > DBL_LONG_MAX will
evaluate to true for some finite extended precision values that are *just*
larger than DBL_LONG_MAX, but nevertheless round to DBL_LONG_MAX rather
than infinity.
Another not-so-bright idea down the drain...
|
msg81458 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-02-09 14:19 |
Fixed (I hope!) in the trunk in r69459. I'll wait for buildbot results
(just in case) and then merge to 2.6, 3.1 and 3.0.
The same test_cmath failure can also be seen on OS X 10.5.6/Intel when
compiling with -fmpmath=387. Annoyingly, the fix above doesn't work here:
it seems that the OS X isinf is buggy. It doesn't seem worth working
around this bug though, since there's little sane reason to be compiling
with -fmpmath=387 in the first place, so it's unlikely that any regular
Python users will encounter this problem.
|
msg81466 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2009-02-09 17:18 |
Merged to py3k in r69465.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:56:42 | admin | set | github: 48825 |
2009-02-09 17:18:53 | mark.dickinson | set | status: open -> closed messages:
+ msg81466 |
2009-02-09 14:19:16 | mark.dickinson | set | resolution: fixed messages:
+ msg81458 versions:
- Python 2.7 |
2009-01-04 23:14:36 | mark.dickinson | set | messages:
+ msg79093 |
2009-01-04 23:01:21 | mark.dickinson | set | messages:
+ msg79092 |
2009-01-04 22:57:09 | mark.dickinson | set | nosy:
+ tim.peters messages:
+ msg79091 |
2009-01-04 22:27:00 | mark.dickinson | set | messages:
+ msg79088 |
2008-12-23 12:54:24 | mark.dickinson | set | files:
+ force_to_memory5.patch messages:
+ msg78231 |
2008-12-15 17:09:12 | mark.dickinson | set | messages:
+ msg77878 |
2008-12-15 16:55:39 | skip.montanaro | set | messages:
+ msg77876 |
2008-12-14 22:03:10 | mark.dickinson | set | messages:
+ msg77841 |
2008-12-14 21:47:45 | rpetrov | set | files:
+ force-inf.c messages:
+ msg77837 |
2008-12-14 21:30:52 | rpetrov | set | messages:
+ msg77834 |
2008-12-14 21:26:38 | rpetrov | set | messages:
+ msg77832 |
2008-12-14 21:22:35 | rpetrov | set | messages:
+ msg77830 |
2008-12-14 21:08:25 | skip.montanaro | set | messages:
+ msg77826 |
2008-12-14 20:41:29 | rpetrov | set | nosy:
+ rpetrov messages:
+ msg77822 |
2008-12-14 18:05:35 | mark.dickinson | set | files:
+ force_to_memory2.patch messages:
+ msg77811 |
2008-12-07 12:42:43 | mark.dickinson | create | |