Skip to content
This repository has been archived by the owner on Dec 6, 2024. It is now read-only.

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
@ksagiyam ksagiyam force-pushed the ksagiyam/hex_interior_facet branch from 53668f8 to 9cc098d Compare September 11, 2024 12:38
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.

@ksagiyam ksagiyam force-pushed the ksagiyam/hex_interior_facet branch from 3030113 to 5c289f2 Compare November 1, 2024 14:07
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 subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants