Skip to content

Unify and fix BoundingBoxtree constructor (C++) - #3545

Merged
jorgensd merged 4 commits into
mainfrom
dokken/unify_and_fix_bb
Dec 2, 2024
Merged

Unify and fix BoundingBoxtree constructor (C++)#3545
jorgensd merged 4 commits into
mainfrom
dokken/unify_and_fix_bb

Conversation

@jorgensd

@jorgensd jorgensd commented Dec 2, 2024

Copy link
Copy Markdown
Member

Reported by @adeebkor.

The constructor:

 /// Constructor
  /// @param[in] mesh The mesh for building the bounding box tree
  /// @param[in] tdim The topological dimension of the mesh entities to
  /// build the bounding box tree for
  /// @param[in] padding Value to pad (extend) the the bounding box of
  /// each entity by.
  BoundingBoxTree(const mesh::Mesh<T>& mesh, int tdim, T padding = 0)
      : BoundingBoxTree::BoundingBoxTree(
            mesh, tdim, range(mesh.topology_mutable(), tdim), padding)
  {

does not work at the moment (and is never tested), as range takes in a reference, not a shared pointer.

To fix this, and follow the rest of DOLFINx, I've unified the constructors using std::optional.

This also removes the None-conversion in the Python layer.

Comment thread cpp/dolfinx/geometry/BoundingBoxTree.h Outdated
@jorgensd
jorgensd enabled auto-merge December 2, 2024 15:00
@jorgensd
jorgensd added this pull request to the merge queue Dec 2, 2024
Merged via the queue into main with commit fa8ba41 Dec 2, 2024
@jorgensd
jorgensd deleted the dokken/unify_and_fix_bb branch December 2, 2024 15:52
schnellerhase pushed a commit to schnellerhase/fenics-dolfinx that referenced this pull request Dec 28, 2024
* Try to unify and fix bb_tree constructors

* Add permissive nanobind

* Apply suggestions from code review
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