-
Notifications
You must be signed in to change notification settings - Fork 159
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
Refactor assemble #1983
Refactor assemble #1983
Conversation
Parloops are now cached on the form. I've done some quick performance checks and (in the pathological case of a very simple kernel and tiny mesh) |
Pending tests and reviews I think this is ready to go. |
Looks good to me! |
684d85d
to
a42c063
Compare
@connorjward I would really appreciate you taking us through these changes and the rationale behind them just so we're all up to date. Maybe at the next firedrake meeting? (Whenever that happens!) Looks like you've done some really nice work here |
Sure, very happy to! I can summarise what I've done pretty straightforwardly though:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been annoying but, I hope, useful and gone through the docstrings to see if everything makes sense to me. See the various comments I've left for more.
commit 9e4f22daacea52b347df170e5c2cd1ba5cd7395e Author: Connor Ward <c.ward20@imperial.ac.uk> Date: Mon Mar 8 15:20:32 2021 +0000 Fix BC bug, I deleted a line by accident... commit 8b89de98e3776828a2a2aa3f18c9235b76bdae7b Author: Connor Ward <c.ward20@imperial.ac.uk> Date: Mon Mar 8 13:51:46 2021 +0000 WIP: Trying to fix assembly_rank commit 0b28cce64c1519d42eb720c16968d65ad38129d3 Author: Connor Ward <c.ward20@imperial.ac.uk> Date: Mon Mar 8 12:02:36 2021 +0000 WIP: test_assemble passes commit 18e30e2fef9696d81b0b9aea0e913c710fc4e09f Author: Connor Ward <c.ward20@imperial.ac.uk> Date: Mon Mar 8 11:43:08 2021 +0000 WIP: Coding done, now just bug fixing needed commit 0b33080 Author: Connor Ward <c.ward20@imperial.ac.uk> Date: Fri Feb 26 16:56:57 2021 +0000 WIP: Add _AssemblyOpts namedtuple to reduce boilerplate
a42c063
to
84178f3
Compare
84178f3
to
3a5a567
Compare
@wence- any chance you could review this at some point? |
This pull request:
_assemble()
. This means thatget_scalar()
,get_vector()
andget_matrix()
only need to returntensor
, rather thantensor, zeros, lambda: tensor
.assemble()
up significantly.The changes will introduce a small amount of overhead where
create_assembly_callable()
is used because we do more logic every time the assembly callable is called. If this overhead is problematic we could also cache the callables that apply the boundary conditions.What do people think? I know the code needs documenting and the caching does not yet work but I wanted to check before spending more time on this.