-
Notifications
You must be signed in to change notification settings - Fork 18
Use overintegration in WADG #354
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
base: main
Are you sure you want to change the base?
Conversation
grudge/op.py
Outdated
| inv_area_elements = 1/project( | ||
| dcoll, dd_base, dd_quad, area_element( | ||
| actx, dcoll, dd=dd_base, | ||
| _use_geoderiv_connection=actx.supports_nonscalar_broadcasting)) |
There was a problem hiding this comment.
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(),)) |
There was a problem hiding this comment.
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
| # Compute 1/J * vec | ||
| def apply_jinv_to_vec(jac_inv, vec): | ||
| return actx.einsum( | ||
| "ei,ej->ei", |
There was a problem hiding this comment.
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
| 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" | ||
| ) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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:
Line 798 in 23fbc05
| if dd_out != dd_in: |
There was a problem hiding this comment.
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)) |
There was a problem hiding this comment.
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?
This change set is intended to update WADG to use overintegration (#173).
TODO:
_apply_inverse_mass_operator