Skip to content

Cache mesh bounding boxes correctly#5180

Open
leo-collins wants to merge 13 commits into
mainfrom
leo/cache-bbox
Open

Cache mesh bounding boxes correctly#5180
leo-collins wants to merge 13 commits into
mainfrom
leo/cache-bbox

Conversation

@leo-collins

Copy link
Copy Markdown
Contributor

After changing mesh coordinates, accessing mesh.bounding_box_coords will now recalculate the bounding boxes.

Added a mesh._check_coordinate_dat_version helper because this sort of thing happens in a few places (bounding boxes, rtree, soon to be distributed rtree)

@connorjward connorjward 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.

The really nice way to do this is:

def cached_property_until(cond: Callable[[Self], bool]):
  def decorator(func):
    def wrapper(self):
      if cond(self):
        return self._cache[some key]
      else:
        return self._cache.setdefault(some key, func(self))

cached_property_until_coords_change = cached_property_until(lambda self: self._saved_coords_dat_version == self.coordinates.dat.dat_version)

so you can have

@cached_property_until_coords_change
def bounding_box_coords(self):
  ...

@connorjward

Copy link
Copy Markdown
Contributor

@achanbour this is very relevant to your work, and you have already probably done something similar. Any comments?

@achanbour

Copy link
Copy Markdown
Contributor

I currently have an attribute that keeps track of the mesh's topology mesh._topology_version (under VertexOnlyMesh only for now). It gets incremented by MeshUpdater (not yet in Firedrake) when the VOM points get redistributed or increase/decrease in number. This does not get incremented when the coordinates change (provided the topology stays the same).

In both regimes (coordinates change OR topology changes), the MeshUpdater clears the cached mesh.bounding_box_coords. I suppose with Leo's change, I can simply remove the line in the updater that clears this cache.

@leo-collins leo-collins marked this pull request as ready for review June 23, 2026 10:15
@leo-collins leo-collins requested a review from connorjward June 23, 2026 15:10
Comment thread firedrake/function.py
Comment thread firedrake/function.py
Comment thread firedrake/function.py Outdated
Comment thread tests/firedrake/regression/test_point_eval_fs.py
Comment thread firedrake/utils.py Outdated
Comment thread firedrake/utils.py Outdated
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.

3 participants