-
Notifications
You must be signed in to change notification settings - Fork 25
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
generalise VariableIndex and FlexiblyIndexed #317
Conversation
bdb7ed6
to
0101a69
Compare
53668f8
to
9cc098d
Compare
facet_orientation = gem.Variable('facet_orientation', (1,), dtype=gem.uint_type) # base mesh entity orientation | ||
self._entity_orientation = { | ||
'+': gem.VariableIndex(gem.Indexed(facet_orientation, (0,))), | ||
'-': gem.VariableIndex(gem.Indexed(facet_orientation, (0,))) |
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.
are these really both zero?
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.
They indeed are. This is for interior_facet_horiz
, so when we walk up the column, facet orientation is always the same as the base cell orientation.
b4a4d16
to
3030113
Compare
gem/node.py
Outdated
def _make_traversal_children(node): | ||
if isinstance(node, (gem.Indexed, gem.FlexiblyIndexed)): | ||
return node.children + node.indirect_children | ||
else: | ||
return node.children |
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.
Note: I'm not sure there is necessarily a better way of doing what you're trying to do, but the fact that the indirect_children
must be handled specially in every traversal is slightly concerning. It's very easy to forget to do it, I would think.
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 think the issue is that there is a gap between the primal DAG and the DAG inside VariableIndex
, as children
of Indexed
object does not include multiindex
.
I at least added some comments in the docstrings.
3030113
to
5c289f2
Compare
hex: enable interior facet integration
bb2e747
to
f33964a
Compare
Reviewed in a Firedrake meeting. |
Generalise:
VariableIndex
to allow forVariableIndex(some_general_expr)
,FlexiblyIndexed
to allow forFlexiblyIndexed((offset_expr, ((index_or_variableindex, stride_expr), )), ...)
.The above changes then let us permute quadrature points based on the provided orientation data, giving us a way to enable interior facet integration on hex.
Potential TODO:
FlexiblyIndexed
).dtype
to eachTerminal
and let operators inheritdtype
from children.Depend on :
FInAT/FInAT#138
firedrakeproject/fiat#83