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

Use overintegration in WADG #354

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

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