Skip to content

Use uw.kdtree.KDTree in mesh.adapt (drop scipy.spatial.cKDTree) - #150

Merged
lmoresi merged 1 commit into
developmentfrom
bugfix/mesh-adapt-uw-kdtree
Apr 28, 2026
Merged

lmoresi merged 1 commit into
developmentfrom
bugfix/mesh-adapt-uw-kdtree

Conversation

@lmoresi

@lmoresi lmoresi commented Apr 28, 2026

Copy link
Copy Markdown
Member

Summary

  • Three sites in `discretisation_mesh.py` used `scipy.spatial.cKDTree` for the mesh-to-mesh variable transfer that runs during `Mesh.adapt`. These were the only `scipy.spatial.cKDTree` usages left in `src/`.
  • Replaced with `uw.kdtree.KDTree` (nanoflann-backed) — the same backend already used by `read_timestep`, swarm migration, and the RBF interpolator path.
  • `.query` API is scipy-compatible — same `(d, i)` return, same `k=` — but defaults to squared distances. Passing `sqr_dists=False` preserves the previous linear-distance semantics so the existing threshold (`1.0e-10` for exact-match) and IDW weighting (`1.0 / (dists + 1e-30)`) are unchanged.

Sites changed

  • `sub_vertex_map` (line 1247) — parent-submesh vertex match.
  • `_re_extract_from_parent` variable IDW transfer (line 1385) — kept k=3 (2D) / k=4 (3D) IDW.
  • `_build_dof_map` (line 1440) — parent-submesh DOF match.

Test plan

  • No `scipy.spatial.cKDTree` references remain in `src/` (verified by grep).
  • `pytest -m "level_1 and tier_a"` on `amr-dev` — 56 passed, 3 skipped, 0 failed.

Notes

This is a minor consistency fix. The bigger architectural item — extracting the global point-location toolkit from `global_evaluate_nd` into a reusable swarm-shaped utility so `mesh.adapt` and `read_timestep` can stop rebuilding rank-local trees redundantly — is recorded in the planning file and is not part of this PR.

Underworld development team with AI support from Claude Code

…mesh.adapt

Three sites in discretisation_mesh.py used scipy.spatial.cKDTree for the
mesh-to-mesh variable transfer that runs during Mesh.adapt:

- sub_vertex_map (line 1247) — parent-submesh vertex match
- _re_extract_from_parent variable IDW transfer (line 1385)
- _build_dof_map (line 1440) — parent-submesh DOF match

These predate the nanoflann switch and were the only scipy.spatial.cKDTree
usages left in src/. uw.kdtree.KDTree (nanoflann-backed) is already used
in read_timestep, swarm migration, and the RBF interpolator path; running
mesh.adapt on the same backend brings consistency and the small
nanoflann-vs-scipy speedup.

The .query API is scipy-compatible — same (d, i) return shape, same k=
parameter — but defaults to squared distances. Passing sqr_dists=False
preserves the previous linear-distance semantics so the existing
threshold (1.0e-10 for exact-match) and IDW weighting
(1.0 / (dists + 1e-30)) are unchanged.

Verified: pytest -m "level_1 and tier_a" on amr-dev passes 56/3/0.

Underworld development team with AI support from Claude Code
Copilot AI review requested due to automatic review settings April 28, 2026 08:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the remaining scipy.spatial.cKDTree usage in Mesh.adapt’s mesh-to-mesh transfer paths by switching to uw.kdtree.KDTree (nanoflann-backed), aligning adapt-time behavior with other parts of the codebase already using the UW KD-tree backend.

Changes:

  • Replaced parent↔submesh vertex matching KD-tree from scipy.spatial.cKDTree to uw.kdtree.KDTree.
  • Replaced adapt-time variable IDW transfer KD-tree from scipy.spatial.cKDTree to uw.kdtree.KDTree while preserving linear-distance semantics via sqr_dists=False.
  • Replaced parent↔submesh DOF matching KD-tree from scipy.spatial.cKDTree to uw.kdtree.KDTree.
Comments suppressed due to low confidence (1)

src/underworld3/discretisation/discretisation_mesh.py:1442

  • Same as _build_vertex_map: KDTree.query() leaves dists as shape (n, 1) for k==1, so matched becomes 2D and indices[matched] will raise (since indices is 1D). Flatten dists to 1D (or update the query call/implementation to return 1D dists when k==1) before using it as a mask.
        tree = uw.kdtree.KDTree(sub_var.coords)
        dists, indices = tree.query(parent_var.coords, sqr_dists=False)
        matched = dists < 1.0e-10

        # indices[matched] maps parent row → sub row
        parent_rows = np.where(matched)[0]
        sub_rows = indices[matched]

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tree = cKDTree(self.X.coords)
dists, indices = tree.query(self.parent.X.coords)
tree = uw.kdtree.KDTree(self.X.coords)
dists, indices = tree.query(self.parent.X.coords, sqr_dists=False)

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

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

uw.kdtree.KDTree.query() returns i reshaped to 1D for k==1, but it leaves d as shape (n, 1). Here that makes matched 2D and then indices[matched] will raise because indices is 1D. Flatten dists (e.g., dists = numpy.asarray(dists).reshape(-1) / dists = dists[:,0]) before building the boolean mask, or adjust the query call/implementation so dists is 1D when k==1 (SciPy-compatible).

Suggested change
dists, indices = tree.query(self.parent.X.coords, sqr_dists=False)
dists, indices = tree.query(self.parent.X.coords, sqr_dists=False)
dists = numpy.asarray(dists).reshape(-1)

Copilot uses AI. Check for mistakes.
Comment on lines +1382 to +1384
tree = uw.kdtree.KDTree(old_coords)
nnn = 3 if self.dim == 2 else 4
dists, indices = tree.query(new_coords, k=nnn)
dists, indices = tree.query(new_coords, k=nnn, sqr_dists=False)

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

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

In the IDW transfer path, old_coords is stored via numpy.array(var.coords) (strips UnitAwareArray metadata), but new_coords = old_var.coords can be unit-aware when model units are active. uw.kdtree.KDTree.query() will raise when the tree was built with dimensionless points but query coords have units. Consider using .coords_nd for both arrays (preferred for internal algorithms) or otherwise ensure both old_coords and new_coords are consistently unitless/unitful before building/querying the tree.

Copilot uses AI. Check for mistakes.
@lmoresi
lmoresi merged commit b3eaa28 into development Apr 28, 2026
5 checks passed
@lmoresi
lmoresi deleted the bugfix/mesh-adapt-uw-kdtree branch April 28, 2026 09:20
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