Skip to content

Refactor bbox to allow performance testing of transpose storage#2400

Merged
sethrj merged 9 commits into
celeritas-project:developfrom
sethrj:bbox-refactor
May 18, 2026
Merged

Refactor bbox to allow performance testing of transpose storage#2400
sethrj merged 9 commits into
celeritas-project:developfrom
sethrj:bbox-refactor

Conversation

@sethrj
Copy link
Copy Markdown
Member

@sethrj sethrj commented May 16, 2026

Many/most of the runtime bounding box algorithms loop over axes on the outside and bounds on the inside. This updates the bounding box internals to allow us to switch the axis/bound storage in the bounding box by writing most code as box.point(bound, axis).

  • Add Extents3 type to allow construction/use by lo/hi pairs for each axis
  • Use point(b, ax) in algorithms
  • Add separate const ref and rvalue ref point accessors to allow ldg(&bbox.point(b, ax)) in future testing

@sethrj sethrj requested a review from elliottbiondo as a code owner May 16, 2026 15:17
@sethrj sethrj added minor Refactoring or minor internal changes/fixes geometry Geometry-related features (geocel) labels May 16, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 16, 2026

Test summary

 6 050 files   9 807 suites   19m 23s ⏱️
 2 311 tests  2 268 ✅  43 💤 0 ❌
34 432 runs  34 245 ✅ 187 💤 0 ❌

Results for commit c5d3a0d.

♻️ This comment has been updated with latest results.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 17, 2026

Codecov Report

❌ Patch coverage is 99.09091% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 87.23%. Comparing base (097a308) to head (94bd2ae).

Files with missing lines Patch % Lines
src/geocel/BoundingBox.hh 98.18% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2400      +/-   ##
===========================================
+ Coverage    87.22%   87.23%   +0.01%     
===========================================
  Files         1396     1396              
  Lines        44025    44050      +25     
  Branches     13791    13303     -488     
===========================================
+ Hits         38400    38427      +27     
+ Misses        4546     4397     -149     
- Partials      1079     1226     +147     
Files with missing lines Coverage Δ
src/orange/BoundingBoxUtils.cc 100.00% <ø> (+2.27%) ⬆️
src/orange/BoundingBoxUtils.hh 97.10% <100.00%> (+0.20%) ⬆️
src/geocel/BoundingBox.hh 98.78% <98.18%> (+3.86%) ⬆️

... and 114 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@elliottbiondo
Copy link
Copy Markdown
Contributor

@sethrj did you look at timing for this?

@sethrj
Copy link
Copy Markdown
Member Author

sethrj commented May 18, 2026

@elliottbiondo This should compile down to the same runtime code I think (some of the utils are slightly different but are only used in setup) so this one should have no performance impact. I'm waiting on ExCL to come back up to run the timing, and additionally check the timing of the bbox transpose.

Comment thread src/geocel/BoundingBox.hh
@sethrj sethrj enabled auto-merge (squash) May 18, 2026 13:54
@sethrj sethrj merged commit 10f9115 into celeritas-project:develop May 18, 2026
40 checks passed
@sethrj sethrj deleted the bbox-refactor branch May 18, 2026 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

geometry Geometry-related features (geocel) minor Refactoring or minor internal changes/fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants