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

DO NOT SQUASH Absorb TSFC #3904

Merged
merged 1,040 commits into from
Dec 6, 2024
Merged

DO NOT SQUASH Absorb TSFC #3904

merged 1,040 commits into from
Dec 6, 2024

Conversation

connorjward
Copy link
Contributor

@connorjward connorjward commented Dec 4, 2024

Happily the changes here are extremely minor.

Goes with firedrakeproject/fiat#104

AndrewWhitmell and others added 30 commits July 9, 2021 16:10
Add flop counting of scheduled impero.
Need to be able to create separate kernel arguments for cell sizes and
generic coefficients.
This doesn't actually work with Firedrake anyway.
Since we pass in the impero_c to construct the kernel, we can just
count flops there.
Push responsibility for construction of the return argument inside the
kernel onto the builder.
The Firedrake code requires a loopy kernel anyway, so cull dead code.
A new "UFLtoGEMCallback" representing the function to dual evaluate is
added. This takes the processed UFL expression and, when called,
performs the necessary translation to gem.

Also add some helpful documentation to partial_indexed.
This rename/refactor makes the purpose of DualEvaluationCallable much
clearer.
Also removes the untested symbolic derivative stuff since Kirby posits
that you can do this with a quadrature rule (like for moments).
This is not supported by FInAT's dual_evaluation anyway.
This fiddling for expression shape not matching element value
shape is already taken care of in the FInAT dual_evaluation method of a
tensor finite element and would now be incorrect. We also no longer give
c_e_d_e a base element in the tensor case.
FInAT element.dual_evaluation now returns (gem_expression, basis_indices)
gem.optimise.contraction now takes care of sum factorisation inside
FInAT so there is no need to tell FInAT about factors. Also not yet
implemented
On tensor product cells, interpolation of Q/DQ elements should sum
factorise to give an operation count that is O(p^{d+1}). Check this
works. Additionally check that interpolation of Vector/TensorElement
costs only the product of the value_shape more flops than the
equivalent scalar element.
Copy link

github-actions bot commented Dec 4, 2024

TestsPassed ✅Skipped ⏭️Failed ❌
Firedrake complex8132 ran6658 passed1474 skipped0 failed

Copy link

github-actions bot commented Dec 4, 2024

TestsPassed ✅Skipped ⏭️Failed ❌
Firedrake real8138 ran7463 passed675 skipped0 failed

pyproject.toml Outdated Show resolved Hide resolved
requirements-git.txt Outdated Show resolved Hide resolved
@connorjward connorjward marked this pull request as ready for review December 5, 2024 11:38
@connorjward connorjward requested a review from ksagiyam December 5, 2024 20:21
@ksagiyam
Copy link
Contributor

ksagiyam commented Dec 6, 2024

Looks good to me. Is it better if we move src/tsfc etc to src/tsfc_old etc to clarify that we are no longer using them? (upon firedrake-update)

@connorjward
Copy link
Contributor Author

Looks good to me. Is it better if we move src/tsfc etc to src/tsfc_old etc to clarify that we are no longer using them? (upon firedrake-update)

Done.

ksagiyam
ksagiyam previously approved these changes Dec 6, 2024
@connorjward connorjward merged commit 6a20368 into master Dec 6, 2024
12 of 16 checks passed
@connorjward connorjward deleted the connorjward/add-tsfc branch December 6, 2024 13:38
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.