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

Remove component tensors #339

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

pbrubeck
Copy link
Contributor

@pbrubeck pbrubeck commented Jan 18, 2025

Consider the bilinear form
a(v, u) = inner(u, v)*dx + inner(div(u), div(v))*dx
where u is a (doubly) Piola mapped tensor in H(div, S) discretized with linear Johnson-Mercier macroelements.

Here we introduce two enhacements targeted to optimize code generation for a(v, u):

  1. We eagerly simplify RefGrad(cellwise_constant) to avoid expressions like RefGrad(Jacobian) -> 0 on affine simplex meshes. These zero terms would arise from the product rule on the doubly Piola mapped tensor.
  2. We eagerly remove ComponentTensors, to make the UFL easier to traverse by the Form compiler.

Since a Form always maps to a scalar, it is always possible to remove ComponentTensor nodes by pushing Indexed inside expressions and simplifying Indexed(ComponentTensor) with the approapiate index replacements.

This PR makes more agressive simplifications within the Indexed constructor, so it's pushed inside sums, and adds a multifunction to handle index replacements. The removal of ComponentTensors is applied as a final preprocessing step in compute_form_data.

@pbrubeck pbrubeck force-pushed the pbrubeck/remove-component-tensors branch from 2e3b737 to 7351d88 Compare January 18, 2025 13:12
@pbrubeck pbrubeck force-pushed the pbrubeck/remove-component-tensors branch from 7351d88 to d328fe5 Compare January 18, 2025 18:11
@pbrubeck pbrubeck force-pushed the pbrubeck/remove-component-tensors branch from d328fe5 to eae33f2 Compare January 19, 2025 10:10
@pbrubeck pbrubeck force-pushed the pbrubeck/remove-component-tensors branch from d83702d to d1b850e Compare January 22, 2025 14:24
@pbrubeck pbrubeck force-pushed the pbrubeck/remove-component-tensors branch 2 times, most recently from 909326f to 83609f8 Compare January 25, 2025 11:23
@pbrubeck pbrubeck force-pushed the pbrubeck/remove-component-tensors branch from 438c594 to 4713c06 Compare January 26, 2025 17:05
…akeproject/ufl into pbrubeck/remove-component-tensors
@jorgensd
Copy link
Member

jorgensd commented Feb 4, 2025

In general this looks sensible to me. Some minor comments/suggestions only.

pbrubeck and others added 2 commits February 4, 2025 22:32
Co-authored-by: Jørgen Schartum Dokken <dokken92@gmail.com>
@pbrubeck pbrubeck force-pushed the pbrubeck/remove-component-tensors branch from 56ba52d to ec00649 Compare February 4, 2025 22:40
"""More precise checks for cellwise constants."""
if is_cellwise_constant(o):
return True
degree = map_expr_dag(self.degree_estimator, o)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've introduced here that is_cellwise_constant logic is based on degree estimation, which is the only place in the code. How does it work, since the degree estimation for geometric quantities calls is_cellwise_constant itself?

def geometric_quantity(self, v):
"""Apply to geometric_quantity.
Some geometric quantities are cellwise constant. Others are
nonpolynomial and thus hard to estimate.
"""
if is_cellwise_constant(v):
return 0
else:
# As a heuristic, just returning domain degree to bump up degree somewhat
return extract_unique_domain(v).ufl_coordinate_element().embedded_superdegree

If there is no infinite loop, could this be used in the generic cell-wise constant check? Or is it that only GradRuleset does not cause infinite loop?

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 first tried to add this logic to the generic is_cellwise_constant in #334, but simplifying grad(cell_wise_constant) on instantiation turns out to be problematic for nested grad expression simply because grad(f) expects f to have a domain and Zero does not have a domain, so grad(grad(grad(linear))) would simplify to grad(Zero).

@@ -653,9 +662,10 @@ def reference_value(self, o):

def reference_grad(self, o):
"""Differentiate a reference_grad."""
if self.is_cellwise_constant(o):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct? Looks like one derivative is lost.

Copy link
Contributor Author

@pbrubeck pbrubeck Feb 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we are simplifying grad(o) where o = grad(f), if o is constant then we should return zero with the correct shape.

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