Skip to content
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

Setting vectors is slow #1485

Open
jrmaddison opened this issue Aug 13, 2019 · 5 comments
Open

Setting vectors is slow #1485

jrmaddison opened this issue Aug 13, 2019 · 5 comments

Comments

@jrmaddison
Copy link
Contributor

jrmaddison commented Aug 13, 2019

The construct Function.vector()[:] = is very slow. The following example:

from firedrake import *

import time

mesh = UnitIntervalMesh(1000000)
space = FunctionSpace(mesh, "Lagrange", 1)
F = Function(space, name="F")
G = Function(space, name="G")

t0 = time.time()
with F.dat.vec_wo as F_v, G.dat.vec_ro as G_v:
    for i in range(10):
        G_v.copy(result=F_v)
print(f"time = {time.time() - t0:.3f} s")

t0 = time.time()
for i in range(10):
    F.vector().set_local(G.vector().get_local())
print(f"time = {time.time() - t0:.3f} s")

t0 = time.time()
for i in range(10):
    F.vector()[:] = G.vector()
print(f"time = {time.time() - t0:.3f} s")

leads, in a test on my machine, to

time = 0.010 s
time = 0.023 s
time = 164.478 s
@dham
Copy link
Member

dham commented Aug 13, 2019

F.vector() is basically only a FEniCS compatibility fudge. The native implementation is F.dat, why not just use that?

@wence-
Copy link
Contributor

wence- commented Aug 13, 2019

This is because the first two do an all-at-once copy, the last one does, for every entry, a getitem on G.vector().

@jrmaddison
Copy link
Contributor Author

I was caught by this when porting FEniCS code, and the cost wasn't obvious until it dominated the calculation. Perhaps a warning could be added? Or this could removed, as it is so slow when used like this.

@dham
Copy link
Member

dham commented Aug 13, 2019

I'm open to the option of putting in a warning on Vector.__setitem__. Can anyone see a downside? I suggest guarding it with a variable so it only triggers once per Vector.

@wence-
Copy link
Contributor

wence- commented Aug 13, 2019

You could also

if isinstance(value, Vector):
    self.dat.data[idx] = value[idx]

In __setitem__?

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

No branches or pull requests

3 participants