Skip to content

ENH: Improve support for POWER7 and POWER8 machines #8134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

gut
Copy link

@gut gut commented Oct 10, 2016

I noticed that those PowerPC machines (as ppc64le architecture) were not passing
all tests of numpy. Mainly because of how long double type is implemented:
https://gcc.gnu.org/wiki/Ieee128PowerPC#A1.3_Sizes

It uses IBM Extended Double and machar algorithm doesn't converge to determine
the long double precision. For that reason I chose to change the long double
size to be 8 bytes (just as double). The consequence of doing that is that
only the precision (mantissa size) is decreased as the exponent remains the
same.

Tests are all passing after I removed long double type of one of them.

Gustavo Serra Scalet added 2 commits October 6, 2016 19:17
Avoid IBM extended double as machar algorithm doesn't converge.

Configured it as opt-in as POWER9 will have native floats of 128 bit
length.
Now `./runtests.py` on PPC64 outputs:
Ran 6273 tests in 96.508s

OK (KNOWNFAIL=6, SKIP=14)
@gut
Copy link
Author

gut commented Oct 10, 2016

This is linked to the following discussion on the mailing list:
https://mail.scipy.org/pipermail/scipy-dev/2016-October/021551.html

@gut
Copy link
Author

gut commented Oct 11, 2016

Do you have any other tests than the ones found on numpy source? Maybe from a major framework that uses numpy which is also supposed to work? Tell if you have any please so I can also verify.

@juliantaylor
Copy link
Contributor

juliantaylor commented Oct 11, 2016

what happens if one just fixes the precision determined by finfo?
there are a couple failures in the stack in debians ppcs, I think most patch it by fixing the precision reported.

@gut
Copy link
Author

gut commented Oct 11, 2016

Fixing the precision? Well, I can take a look at that. If you can send me these patches I'd appreciate.

Just to be clear about the current behavior:

This PR outputs:

>>> np.finfo(np.longdouble)
finfo(resolution=1e-15, min=-1.7976931348623157e+308, max=1.7976931348623157e+308, dtype=float64)

And the master outputs:

>>> np.finfo(np.longdouble)
/home/gut/python/venv3.5/lib/python3.4/site-packages/numpy-1.12.0.dev0+33d850d-py3.4-linux-ppc64le.egg/numpy/core/machar.py:127: RuntimeWarning: overflow encountered in add
  a = a + a
/home/gut/python/venv3.5/lib/python3.4/site-packages/numpy-1.12.0.dev0+33d850d-py3.4-linux-ppc64le.egg/numpy/core/machar.py:129: RuntimeWarning: invalid value encountered in subtract
  temp1 = temp - a
/home/gut/python/venv3.5/lib/python3.4/site-packages/numpy-1.12.0.dev0+33d850d-py3.4-linux-ppc64le.egg/numpy/core/machar.py:138: RuntimeWarning: invalid value encountered in subtract
  itemp = int_conv(temp-a)
/home/gut/python/venv3.5/lib/python3.4/site-packages/numpy-1.12.0.dev0+33d850d-py3.4-linux-ppc64le.egg/numpy/core/machar.py:162: RuntimeWarning: overflow encountered in add
  a = a + a
/home/gut/python/venv3.5/lib/python3.4/site-packages/numpy-1.12.0.dev0+33d850d-py3.4-linux-ppc64le.egg/numpy/core/machar.py:164: RuntimeWarning: invalid value encountered in subtract
  temp1 = temp - a
/home/gut/python/venv3.5/lib/python3.4/site-packages/numpy-1.12.0.dev0+33d850d-py3.4-linux-ppc64le.egg/numpy/core/machar.py:171: RuntimeWarning: invalid value encountered in subtract
  if any(temp-a != zero):
finfo(resolution=1e-75, min=--9.22337203471e+18, max=-9.22337203471e+18, dtype=float128)

@gut
Copy link
Author

gut commented Oct 11, 2016

oh, I just noticed some related issues, like #7940 and #2669

@gut
Copy link
Author

gut commented Oct 11, 2016

so you are aware of the ibm extended double implementation which uses 2 doubles to represent a long double. Exponent is the same as double but it has doubled the mantissa size, so the precision is better.
I guess that the easiest way to keep compatibility currently is to downcast it to a simple double, as it's a IEEE type and machar detection works fine.

Do you have a requirement to use his IBM Extended Double on PPC64? It could be hardcoded as #2669 suggested.

@juliantaylor
Copy link
Contributor

yes we are aware of the different representation, there is specific code to support it. It would be good if we can keep it by just fixing the wrong np.finfo output and maybe adapting some tests boundaries.
If we really have to disable its use we also should remove all support code.

@charris charris changed the title Improve support for POWER7 and POWER8 machines ENH: Improve support for POWER7 and POWER8 machines Oct 13, 2016
@charris
Copy link
Member

charris commented Oct 14, 2016

I note that the long double format is detected during build, to include IBM both little endian and big endian, so we could expose that information. I'd like to expose the various macros generated during build in any case.

@gut
Copy link
Author

gut commented Oct 25, 2016

@juliantaylor : But then I guess that the algorithm in machar would need to be fix, right? The little I understood from that algorithm, it looked like it'd be a major update in order to be able to detect the peculiar precision of IBM extended double, right?

Only by fixing np.finfo(np.longdouble)'s output would not suppress the RuntimeWarnings raised by machar algorithm, which is also a concern.

@gut
Copy link
Author

gut commented Oct 28, 2016

How should we then proceed?

@gut
Copy link
Author

gut commented Nov 3, 2016

@charris @juliantaylor ping?

@charris
Copy link
Member

charris commented Nov 4, 2016

@juliantaylor If the Debian patches fix the problem, I'd like to get hold of them. In general, I'd like to fix finfo if that does the trick.

@gut
Copy link
Author

gut commented Nov 14, 2016

@charris finfo output relies on a machar call, right? Or do you intend in fixing finfo somewhere else?

@charris
Copy link
Member

charris commented Nov 14, 2016

I actually think that we should dump machar and hard code the values. Enough info is gathered during the build that we know what types we have, which are basically IEEE and IBM double_double long doubles. We only claim to support IEEE in any case but can probably make the IBM type work. However, we need to expose a bit more of the build information, so that needs to be the first order of business.

@charris
Copy link
Member

charris commented Feb 3, 2017

Should be closed by #8504.

@charris charris closed this Feb 3, 2017
@gut
Copy link
Author

gut commented Feb 6, 2017

I confirm there is no issue when running numpy tests on python 3.5.2 and 2.7.12.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants