Simplify tabulation through dolfinx::FiniteElement - #1565
Merged
Conversation
garth-wells
force-pushed
the
dokken/finite-element-tabulate
branch
from
June 22, 2021 06:43
4786ca4 to
1bdf269
Compare
jorgensd
commented
Jun 22, 2021
IgorBaratta
reviewed
Jun 22, 2021
| /// dimension] containing the points at the reference element | ||
| /// @param[in] order The number of derivatives (up to and including this | ||
| /// order) to tabulate for. | ||
| void tabulate(xt::xtensor<double, 4>& values, const xt::xtensor<double, 2>& X, |
Member
There was a problem hiding this comment.
Should we also change the order of parameters in CoordinateElement::tabulate for consistency?
garth-wells
reviewed
Jun 22, 2021
Comment on lines
+305
to
+306
| auto basis_reference_values = xt::view(basis_derivatives_reference_values, | ||
| 0, xt::all(), xt::all(), xt::all()); |
Member
There was a problem hiding this comment.
A little issue is that this will be copied further down in each loop iteration when passed to element->transform_reference_basis since transform_reference_basis expects a xt::xarray.
Member
Author
There was a problem hiding this comment.
That is not correct?
Transform reference basis takes an xtensor:
dolfinx/cpp/dolfinx/fem/FiniteElement.cpp
Lines 200 to 208 in 13b06ce
and
map_push_forward_m takes an xtensor: https://github.com/FEniCS/basix/blob/266c9b486d6a8e6d00c118cb05f499690cd98679/cpp/basix/finite-element.h#L317
Member
There was a problem hiding this comment.
Yes, it takes a xt::xtensor<double, 3>, but xt::view isn't a xt::xtensor<double, 3> hence a copy will be made.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As DOLFINx uses
xtensorthere is no point of distinguishing between tabulation of 0th order and nth order derivatives of the basis function.Also makes it possible to tabulate the derivative of basis function in DOLFINx (useful for custom kernels).