Various nanobind memory fixes#4299
Merged
Merged
Conversation
- la.petsc.create_matrix: use nb::steal instead of nb::borrow, which leaked the Mat/Python wrapper on every call. - Scatterer.scatter_fwd: fix a bounds check that compared local_data's size against remote_indices().size() instead of local_indices().size(), the array it's actually indexed by. - mesh.Geometry.__init__: validate 0 < shape[1] <= 3 before padding to 3D; an (N, >3) input previously wrote past the padded buffer. - geometry point-distance functions (compute_distance_gjk, compute_distances_gjk, squared_distance, determine_point_ownership): add a shared validation helper so a point array's last dimension is checked against 3, instead of being assumed. - fem.FiniteElement.T_apply/Tt_apply/Tt_inv_apply: guard against division by zero when cell_permutations is empty. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Geometry.dofmaps: fix reference_internal being a no-op on ndarrays appended to an nb::list return value (it only applies to the outer return, not per-element). Take nb::object self and pass it as the explicit owner of each ndarray, restoring genuine zero-copy views instead of copying on every access. - create_mesh (both overloads): drop ~30 lines of duplicated Python partitioner marshalling logic, reusing the existing part::impl::create_cell_partitioner_cpp helper already used by create_interval/create_rectangle/create_box. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The dim > 3 check regressed to only checking the upper bound, silently allowing shape1 == 0 (a degenerate, dimensionless Geometry) even though the error message still documents 0 < dim <= 3. Not an out-of-bounds issue (the padding loop simply runs zero times), but inconsistent with the stated and intended constraint. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nal to original_cell_indices The setter for original_cell_index unconditionally resized to 1, silently truncating data for a mixed-topology mesh whose getter already rejects that case. original_cell_indices was missing reference_internal, causing an unnecessary copy on every access.
assert(bc) was stripped by NDEBUG in Release builds, letting a None DirichletBC in a Python-supplied list dereference a null pointer; replaced with a real throw. discrete_curl/discrete_gradient/ interpolation_matrix leaked the newly created Mat if building the operator threw; now destroyed on the exception path. create_matrix no longer duplicates the Mat type-caster's take_ownership logic by hand, and the repeated IndexMap-pair conversion is factored into a single to_index_map_refs helper.
Only Mat and Vec had a registered type_caster, so create_index_sets and NewtonSolver.krylov_solver had to hand-roll the same PyPetscXxx_New/PetscObjectDereference dance; added IS and KSP casters and simplified both call sites to use them. Also widened the from_cpp rv_policy fallback to treat automatic/reference_internal the same as reference/automatic_reference instead of silently returning a null handle.
from_cpp only handled automatic/automatic_reference/reference_internal, silently returning a null handle for reference/take_ownership (the mirror of the gap just fixed in caster_petsc.h). Since MPICommWrapper always wraps a plain handle by value, invert the check to allow every policy except none. Also mark the single-argument MPI_Comm constructor explicit to prevent accidental implicit conversions.
_V begins with an underscore followed by an uppercase letter, which is reserved to the implementation; renamed to VDecay. Also assert that the caller-supplied shape's element count matches the vector's actual size, since a mismatch would silently produce a numpy view that reads past the underlying buffer.
Each of these previously relied only on assert() (compiled out under the Release/NDEBUG build used by the shipped wheel) or had no check at all, letting out-of-range indices from Python reach raw vector/array writes: - common.cpp: create_sub_index_map's indices array - refinement.cpp: transfer_cell_meshtag/transfer_facet_meshtag's parent_cell/parent_facet arrays - refinement.h: refine()'s edges array - la.h: MatrixCSR.add/set's rows array and x/rows/cols size consistency - graph.h: AdjacencyList(data, offsets) constructor's offsets consistency (non-decreasing, starts at 0, matches data length)
The bounds check added for heap-corruption hardening compared rows against num_all_rows() unconditionally, but that count is only correct when the insertion block size (bs) matches the matrix's own block size. When they differ (e.g. inserting unblocked bs=1 indices into a blocked bs=2 matrix, or vice versa), the valid index range must be scaled by the ratio between the two block sizes, mirroring the three cases MatrixCSR::add/set dispatch on internally. Fixes CI failures in test_matrix_csr.py::test_add and ::test_set_blocked. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
garth-wells
marked this pull request as ready for review
July 18, 2026 14:09
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
1 participant
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.
Various nanobind memory fixes and improvements, and style improvements.