Skip to content

Use Index_CreateWithArray to build mesh spatial index#4865

Open
leo-collins wants to merge 10 commits intomainfrom
leo/profile-vom
Open

Use Index_CreateWithArray to build mesh spatial index#4865
leo-collins wants to merge 10 commits intomainfrom
leo/profile-vom

Conversation

@leo-collins
Copy link
Contributor

@leo-collins leo-collins commented Feb 5, 2026

Before we were looping over bounding box coords and calling Index_InsertData one by one which was a big bottleneck. libspatialindex has a function which can build a spatial index in one go from an array: https://libspatialindex.org/en/latest/doxygen/sidx__api_8cc.html#a36737b1629d8a96458e7f437484f5a84

Building the spatial index in this example went from ~15s on my machine to ~0.9s

from firedrake import *

mesh = UnitSquareMesh(500, 500)
mesh.spatial_index

Copy link
Contributor

@connorjward connorjward left a comment

Choose a reason for hiding this comment

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

This is fantastic. So often performance improvements come down to "we were doing something quite silly before".

@achanbour this PR is likely of interest to you, if you haven't seen it yet.

cells_ignore)
with PETSc.Log.Event("cells_data_as"):
cells_data = cells.ctypes.data_as(ctypes.POINTER(ctypes.c_int))
with PETSc.Log.Event("ref_cell_dists_l1_as"):
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like an unnecessary number of events to be timing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove these before merging

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah OK. In future if you ask me for a review please tell me if I can ignore bits of it.

Copy link
Contributor

Choose a reason for hiding this comment

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

And you should definitely leave some of them in there

@leo-collins leo-collins marked this pull request as ready for review February 6, 2026 17:04
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