Improve performance of multivector arithmetic by avoiding copies

[eric-wieser]

Multiplications become roughly 20% faster, and addition 33% faster.
There's now a new copy argument to the MultiVector constructor.

Some timings:

Edit: These timings are out of date, and include the effects of #283 which used to be part of this PR. We should retime before merging

After:

In [1]: from clifford import Cl
In [2]: layout, _ = Cl(5)
In [3]: a = layout.randomMV(); b = layout.randomMV()

In [4]: %timeit a * b
The slowest run took 4.76 times longer than the fastest. This could mean that an intermediate result is being cached.
20.4 µs ± 11.3 µs per loop (mean ± std. dev. of 7 runs, 1 loop each)

In [5]: %timeit a * b
8.64 µs ± 341 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

In [6]: %timeit a * b
9.23 µs ± 606 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

In [7]: %timeit a + b
3.9 µs ± 250 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

Before:

In [1]: from clifford import Cl
In [2]: layout, _ = Cl(5)
In [3]: a = layout.randomMV(); b = layout.randomMV()

In [4]: %timeit a * b
The slowest run took 4.37 times longer than the fastest. This could mean that an intermediate result is being cached.
18.6 µs ± 14.2 µs per loop (mean ± std. dev. of 7 runs, 1 loop each)

In [5]: %timeit a * b
11.7 µs ± 759 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

In [6]: %timeit a * b
11.3 µs ± 660 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

In [7]: %timeit a + b
6.11 µs ± 871 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

[review-notebook-app]

Check out this pull request on 

You'll be able to see Jupyter notebook diff and discuss changes. Powered by ReviewNB.

[eric-wieser]

I'm wondering if actually we should just change the default to copy=False. I haven't yet found an internal call that would break if we did this.

[lgtm-com]

This pull request introduces 1 alert and fixes 6 when merging 31ff317 into a5c9431 - view on LGTM.com

new alerts:

• 1 for Syntax error

fixed alerts:

• 3 for Variable defined multiple times
• 2 for Unused import
• 1 for 'import *' may pollute namespace

[hugohadfield]

This might cause problems as the unit_scalar is a constant scoped to the whole file

[eric-wieser]

Indeed - however, I don't think this is any worse than the fact that clifford.g3c.e3 is global, and I could do something careless like

my_vector = e3
my_vector.value[:] = e2.value  # whoops, now e3 == e2

Perhaps I'll put my patch in to fix that first - the blades that are part of the predefined algebras should definitely be readonly. I'm not sure if layout.blades should be though.

[hugohadfield]

omg I hate it

[hugohadfield]

Yeah I have no strong feeling about layout.blades but the predefined blades definitely have to be readonly

[eric-wieser]

eo.value[:] = layout.scalar.value[:]
print("Your CGA is now somewhat PGA")

[eric-wieser]

Let's not merge this till we:

• Decide on Can we change MultiVector(layout, arr) to no longer copy arr? #282
• Re-profile it in isolation