Skip to content

Conversation

@MTCam
Copy link
Collaborator

@MTCam MTCam commented Jul 19, 2024

This change set is intended to update WADG to use overintegration (#173).

TODO:

  • Add tests
  • Recombine multiple stages into 2 loopy calls
  • Refactor back into original _apply_inverse_mass_operator

grudge/op.py Outdated
Comment on lines 852 to 855
inv_area_elements = 1/project(
dcoll, dd_base, dd_quad, area_element(
actx, dcoll, dd=dd_base,
_use_geoderiv_connection=actx.supports_nonscalar_broadcasting))
Copy link
Owner

Choose a reason for hiding this comment

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

The area element is already nonlinear. Its computation should take place on the target discretization.

"ij,ej->ei",
ref_inv_mass,
vec,
tagged=(FirstAxisIsElementsTag(),))
Copy link
Owner

Choose a reason for hiding this comment

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

Please use axis tags, not this. Also below.

grudge/op.py Outdated
Comment on lines 865 to 868
# Compute 1/J * vec
def apply_jinv_to_vec(jac_inv, vec):
return actx.einsum(
"ei,ej->ei",
Copy link
Owner

Choose a reason for hiding this comment

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

Do you mean just * here, i.e. element-wise multiplication? This einsum will sum over each element!

grudge/op.py Outdated
Comment on lines 832 to 837
if dd_out != dd_in:
raise ValueError(
"Cannot compute inverse of a mass matrix mapping "
"between different element groups; inverse is not "
"guaranteed to be well-defined"
)
Copy link
Owner

Choose a reason for hiding this comment

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

Why have both dd_in and dd_out, if having them differ is not appropriate?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wondered that myself :). Copied from the original _apply_inverse_mass:

if dd_out != dd_in:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed up in 45d6388

grudge/op.py Outdated
for grp, vec_i in zip(discr_base.groups, stage3)
]

return DOFArray(actx, data=tuple(group_data))
Copy link
Owner

Choose a reason for hiding this comment

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

Why build many intermediate DOFArrays as opposed to constructing the final one in one go?

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.

2 participants