Optimize circumsphere and triangulation.py

[philippeitis]

To speed up circumsphere, I directly called numpy's determinant function, as fast_det defaults to this case for >= 4 dimensions, which is always the case for circumsphere. I also directly took the norm, as this again defaults to sqrt(np.dot(vec * vec)) in fast_det for 4 dimensions. I also removed np.delete in favour of masked indices, which means that numpy doesn't need to allocate new arrays for each determinant.

For circumsphere, the speedup is about 33% on a four dimensional sphere (1.35x faster):

(50000 runs on arbitrary coordinates)

opt  circumsphere: 8.907416s
nopt circumsphere: 13.325046s
opt  circumsphere: 7.633841s
nopt circumsphere: 12.151468s

For a five dimensional sphere, the difference is more obvious, at around 50% (2x faster):
(50000 runs on arbitrary coordinates)

opt  circumsphere: 11.039010s
nopt circumsphere: 19.448841s
opt  circumsphere: 10.594215s
nopt circumsphere: 18.614510s

I also added general optimizations by directly importing all numpy functions - for hot loops, this means that Python doesn't have to look up the function repeatedly, which can save a fair amount of time.

[basnijholt]

Thanks a lot for doing these optimizations!

You can easily fix all the style issues by running pre-commit run --all (first install using pip install pre-commit).

I've made #246 to make this more obvious in the future.

[akhmerov]

Should we use @ instead of dot? I might be wrong, but it seems either faster or same speed, and more readable.

[akhmerov]

While most of the names that we import have no collision with built-ins, abs does override the built-in one. Would it be a good idea to import abs as np_abs?

Is making all names available in the module namespace an important optimization?

[philippeitis]

While most of the names that we import have no collision with built-ins, abs does override the built-in one. Would it be a good idea to import abs as np_abs?

Is making all names available in the module namespace an important optimization?

As a quick demo:

>python -m timeit -s"from math import sqrt" "sqrt(5.6)"
5000000 loops, best of 5: 69.4 nsec per loop

>python -m timeit -s"import math" "math.sqrt(5.6)"
5000000 loops, best of 5: 94.8 nsec per loop

A 20-30% performance boost from direct imports is pretty nice to have, though this may vary for specific functions and their overhead.

[philippeitis]

Should we use @ instead of dot? I might be wrong, but it seems either faster or same speed, and more readable.

The issue with @ is that it doesn't work if the argument is a regular list (I think a lot of calls to fast norm aren't numpy arrays). Also, it seems to be slower, at least from my testing:

>python -m timeit -s"from numpy import array;vect = array([1,2,3,4])" "vect @ vect"
500000 loops, best of 5: 879 nsec per loop

>python -m timeit -s"from numpy import dot, array; vect = array([1,2,3,4])" "dot(vect, vect)"
500000 loops, best of 5: 603 nsec per loop

[philippeitis]

Azure seems to be throwing this error which is causing the linting tests to fail:

Traceback (most recent call last):
  File "/home/vsts/.cache/pre-commit/repom1wn1zn2/py_env-python3.7/bin/black", line 5, in <module>
    from black import patched_main
  File "/home/vsts/.cache/pre-commit/repom1wn1zn2/py_env-python3.7/lib/python3.7/site-packages/black.py", line 15, in <module>
    import regex as re
  File "/home/vsts/.cache/pre-commit/repom1wn1zn2/py_env-python3.7/lib/python3.7/site-packages/regex.py", line 402, in <module>
    import _regex_core
  File "/home/vsts/.cache/pre-commit/repom1wn1zn2/py_env-python3.7/lib/python3.7/site-packages/_regex_core.py", line 21, in <module>
    import regex._regex as _regex
ModuleNotFoundError: No module named 'regex._regex'; 'regex' is not a package

[basnijholt]

That failure is because of psf/black#1207.

I imagine that will be fixed soon.

[jbweston]

LGTM, and thank you for adding an extra test!

[jbweston]

I merged this manually (so that we could get a rebase but also an explicit merge commit, which GitHub does not allow, for some reason), but it seems that GitHub did not register this as closing this PR.

@philippeitis your changes are merged in, but I will have to "close" this PR.

[jbweston]

@philippeitis Let me know how you'd like me to credit you in the AUTHORS file (your GH profile does not have your real name).

[philippeitis]

@jbweston Happy to see these changes merged in. If you'd like to credit me in the AUTHORS file, my name is Philippe Solodov.