Connectivity peakmem benchmarks - #1661
Merged
Merged
Conversation
cmdupuis3
added
the
benchmarking
ASV BenchmarkingBenchmark Comparison ResultsBenchmarks that have improved:
Benchmarks that have stayed the same:
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Drop the GridBenchmark template, whose full cached_grid setup was always overridden, and root MinimalGridBenchmark on CachedFixtures directly so the suite only touches the parent-preloaded topology fixtures. Pin numba threads around the tracemalloc samples, as the other suites do, since edge_node and face_edge are built by parallel kernels. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cmdupuis3
marked this pull request as ready for review
September 29, 2026 21:01
dylannelson
reviewed
Oct 1, 2026
dylannelson
approved these changes
Oct 1, 2026
dylannelson
left a comment
dylannelson
left a comment
Member
There was a problem hiding this comment.
Everything works as stated and seeing no concerns. Approving with a comment as it may be nice to import the connectivities from the conventions file as opposed to re defining them here. But if there's a good reason for re defining them feel free to resolve the comment and push forward. We talked about potentially changing some of the conventions file so this would allow us to change in only one place rather than multiple, but that's a bit further down the line
Sevans711
approved these changes
Oct 2, 2026
Sevans711
left a comment
Sevans711
left a comment
Collaborator
There was a problem hiding this comment.
Read through the changes and they all look reasonable. It's a bit strange to pass something around by editing a class attribute instead of an instance attribute, but the docstrings/comments clearly explain the motivation, that this decision was made to help deal with one of ASV's many quirks, so I'm happy to let this proceed as-is. It also looks like the connectivity benchmark results are all working properly, based on the ASV bot comment in this PR. Approving!
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
3 participants
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.
Closes #1660
Overview
Two sets of memory benchmarks, tracking allocations in isolation and total peakmem usage.
ConnectivityTracemalloc results from Derecho:
(face_edge is a flat 11.4k, face_node is a flat 2.54k)
PR Checklist
General
Testing & Benchmarking
Documentation
AI Disclosure
AI Usage: Claude Opus 5