Skip to content

Conversation

@schnellerhase
Copy link
Contributor

@schnellerhase schnellerhase commented Nov 29, 2024

Together with #3509 fixes #3483.

@schnellerhase schnellerhase marked this pull request as ready for review November 29, 2024 21:44
Copy link
Member

@garth-wells garth-wells left a comment

Choose a reason for hiding this comment

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

The wrapping of the C++ FiniteElement needs to be sorted out. See comment on cached_property in the discussion.

@schnellerhase
Copy link
Contributor Author

The discussed case has been changed in 02d03f2.

@jhale jhale added this pull request to the merge queue Dec 5, 2024
Merged via the queue into FEniCS:main with commit fcc7719 Dec 5, 2024
20 checks passed
cell_permutations: Permutation data for the cell.
dim: Number of columns in `data`.
Note:
Copy link
Member

Choose a reason for hiding this comment

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

@schnellerhase This is not correct. These functions have loop access in the python wrapper.

Copy link
Member

Choose a reason for hiding this comment

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

The type hint should be npt.NDArray[np.int32].

const std::size_t data_per_cell
= x.size() / cell_permutations.size();
std::span<T> x_span(x.data(), x.size());
std::span<const std::uint32_t> perm_span(
cell_permutations.data(), cell_permutations.size());
for (std::size_t i = 0; i < cell_permutations.size(); i++)
{
self.Tt_apply(x_span.subspan(i * data_per_cell, data_per_cell),
perm_span[i], dim);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the catches and fixes - my bad!

Exposed for testing. Function is not vectorised across multiple cells. Please see
``basix.numba_helpers`` for performant versions.
"""
self._cpp_object.Tt_apply(x, cell_permutations, dim)
Copy link
Member

Choose a reason for hiding this comment

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

This is also wrong, as this should apply the inverse transform.

@schnellerhase schnellerhase deleted the add_finiteelement_wrapper branch December 7, 2024 09:56
schnellerhase added a commit to schnellerhase/fenics-dolfinx that referenced this pull request Dec 28, 2024
* Start to wrap FiniteElement

* Ruff

* Ufl naming

* More wrapping

* ruff

* last one?

* Misse done

* nomatching

* Add docstrings

* Add tpye hints

* Remove returns for properties

* Add argument doc strings

* more document args

* Rename: dtype -> FiniteElement_dtype

* finite_element -> finiteelement

* Ruff

* Apply suggestion

* Switch to cached_propert for element access

* FixreStructured text

* Fix return type descriptions
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.

Python wrappers for AdjacencyList and FiniteElement missing

5 participants