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

generalise VariableIndex and FlexiblyIndexed #317

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

ksagiyam
Copy link
Contributor

@ksagiyam ksagiyam commented Sep 3, 2024

Generalise:

  • VariableIndex to allow for VariableIndex(some_general_expr),
  • FlexiblyIndexed to allow for FlexiblyIndexed((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:

  • enable full index arithmetic (and retire FlexiblyIndexed).
  • attach dtype to each Terminal and let operators inherit dtype from children.

Depend on :
FInAT/FInAT#138
firedrakeproject/fiat#83

@ksagiyam ksagiyam force-pushed the ksagiyam/hex_interior_facet branch 3 times, most recently from bdb7ed6 to 0101a69 Compare September 9, 2024 01:09
@ksagiyam ksagiyam marked this pull request as ready for review September 9, 2024 11:49
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,)))
Copy link
Member

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?

Copy link
Contributor Author

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.

@ksagiyam ksagiyam force-pushed the ksagiyam/hex_interior_facet branch 5 times, most recently from b4a4d16 to 3030113 Compare September 12, 2024 18:32
gem/node.py Outdated
Comment on lines 103 to 108
def _make_traversal_children(node):
if isinstance(node, (gem.Indexed, gem.FlexiblyIndexed)):
return node.children + node.indirect_children
else:
return node.children
Copy link
Contributor

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.

Copy link
Contributor Author

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.

hex: enable interior facet integration
@ksagiyam ksagiyam force-pushed the ksagiyam/hex_interior_facet branch 2 times, most recently from bb2e747 to f33964a Compare November 6, 2024 14:03
@ksagiyam ksagiyam merged commit 1944432 into master Nov 6, 2024
4 checks passed
@ksagiyam ksagiyam deleted the ksagiyam/hex_interior_facet branch November 6, 2024 14:07
@ksagiyam
Copy link
Contributor Author

ksagiyam commented Nov 6, 2024

Reviewed in a Firedrake meeting.

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

Successfully merging this pull request may close these issues.

3 participants