Skip to content

Store BIH metadata and write to JSON#2244

Open
elliottbiondo wants to merge 1 commit intoceleritas-project:developfrom
elliottbiondo:bih_diagnostic
Open

Store BIH metadata and write to JSON#2244
elliottbiondo wants to merge 1 commit intoceleritas-project:developfrom
elliottbiondo:bih_diagnostic

Conversation

@elliottbiondo
Copy link
Contributor

@elliottbiondo elliottbiondo commented Feb 6, 2026

This MR store the number of finite/nonfinite volume and maximum embedding depth for the BIH of each universe. This will facilitate optimization of BIH tuning parameters.

@elliottbiondo elliottbiondo requested a review from sethrj as a code owner February 6, 2026 22:46
@elliottbiondo elliottbiondo marked this pull request as draft February 6, 2026 22:46
@codecov
Copy link

codecov bot commented Feb 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.88%. Comparing base (de8db3d) to head (00dac83).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2244      +/-   ##
===========================================
+ Coverage    86.82%   86.88%   +0.05%     
===========================================
  Files         1330     1331       +1     
  Lines        42027    42112      +85     
  Branches     12949    12954       +5     
===========================================
+ Hits         36491    36587      +96     
+ Misses        4333     4321      -12     
- Partials      1203     1204       +1     
Files with missing lines Coverage Δ
src/orange/OrangeData.hh 97.14% <ø> (ø)
src/orange/detail/BIHBuilder.cc 100.00% <100.00%> (ø)
src/orange/detail/BIHBuilder.hh 0.00% <ø> (ø)

... and 14 files with indirect coverage changes

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

@github-actions
Copy link

github-actions bot commented Feb 6, 2026

Test summary

 5 913 files   9 451 suites   18m 19s ⏱️
 2 151 tests  2 123 ✅  28 💤 0 ❌
32 338 runs  32 180 ✅ 158 💤 0 ❌

Results for commit 00dac83.

@elliottbiondo elliottbiondo changed the title Store and output BIH metadata Store BIH metadata and write to JSON Feb 7, 2026
OrangeParamsOutput out(this->geometry());
EXPECT_JSON_EQ(
R"json({"_category":"internal","_label":"orange","scalars":{"num_univ_levels":3,"max_faces":8,"max_intersections":14,"max_csg_levels":1,"tol":{"abs":1e-05,"rel":1e-05}},"sizes":{"bih":{"bboxes":24,"inner_nodes":9,"leaf_nodes":16,"local_volume_ids":13},"connectivity_records":13,"daughters":6,"fast_real3s":0,"local_surface_ids":20,"local_volume_ids":18,"logic_ints":31,"obz_records":0,"real_ids":13,"reals":46,"rect_arrays":0,"simple_units":7,"surface_types":13,"transforms":6,"universe_indexer":{"surfaces":8,"volumes":8},"univ_indices":7,"univ_types":7,"volume_ids":24,"volume_instance_ids":24,"volume_records":24}})json",
R"json({"_category":"internal","_label":"orange","bih_metadata":{"max_depth":[5,0,0,4,2,0,0],"num_finite_bboxes":[6,0,0,4,2,0,0],"num_nonfinite_bboxes":[1,0,0,0,0,0,0]},"scalars":{"logic":"postfix","max_csg_levels":1,"max_faces":8,"max_intersections":14,"num_univ_levels":3,"tol":{"abs":1e-05,"rel":1e-05}},"sizes":{"bih":{"bboxes":24,"inner_nodes":9,"leaf_nodes":16,"local_volume_ids":13},"connectivity_records":13,"daughters":6,"fast_real3s":0,"local_surface_ids":20,"local_volume_ids":18,"logic_ints":31,"obz_records":0,"real_ids":13,"reals":46,"rect_arrays":0,"simple_units":7,"surface_types":13,"transforms":6,"univ_indices":7,"univ_types":7,"universe_indexer":{"surfaces":8,"volumes":8},"volume_ids":24,"volume_instance_ids":24,"volume_records":24}})json",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before merging I'd like to investigate why several universes here have no bboxes; not sure if that makes sense.

Copy link
Member

Choose a reason for hiding this comment

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

Also not ideal is that there are so many trees with depth == num_finite...

Note that the input file is built by UnitProto.test.cc, and when you run that, it spits out inputbuilder-hierarchy.csg.json, which contains all the bzones and volumes for each universe. That lets us see that the no-bbox cases are for "leaf" nodes that have only a single background volume and no interior volumes.

@elliottbiondo elliottbiondo marked this pull request as ready for review February 7, 2026 00:55
@elliottbiondo
Copy link
Contributor Author

@sethrj RTR

ItemRange<LocalVolumeId> inf_vol_ids;

//! The metadata for this tree
MetadataId metadata_id;
Copy link
Member

Choose a reason for hiding this comment

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

I don't think the metadata needs its own ID since they're all scalars and each tree has a single metadata item (there's no shuffling). Just store the values directly on the tree.

size_type num_nonfinite_bboxes;
//! The depth of the most embedded leaf node. This has a value of 1
//! when the root node is a leaf.
size_type max_depth;
Copy link
Member

Choose a reason for hiding this comment

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

I think depth, not "max depth"?

//! The number of finite bounding boxes in the tree
size_type num_finite_bboxes;
//! The number of nonfinite bounding boxes in the tree
size_type num_nonfinite_bboxes;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this could be more descriptive of how "nonfinite" is different from infinite (I guess they're both infinite in volume, but nonfinite is just semi-infinite on at least one dimension) ? Or perhaps how they're treated differently from finite bboxes? e.g., num_ignored_bboxes or num_global_bboxes

OrangeParamsOutput out(this->geometry());
EXPECT_JSON_EQ(
R"json({"_category":"internal","_label":"orange","scalars":{"num_univ_levels":3,"max_faces":8,"max_intersections":14,"max_csg_levels":1,"tol":{"abs":1e-05,"rel":1e-05}},"sizes":{"bih":{"bboxes":24,"inner_nodes":9,"leaf_nodes":16,"local_volume_ids":13},"connectivity_records":13,"daughters":6,"fast_real3s":0,"local_surface_ids":20,"local_volume_ids":18,"logic_ints":31,"obz_records":0,"real_ids":13,"reals":46,"rect_arrays":0,"simple_units":7,"surface_types":13,"transforms":6,"universe_indexer":{"surfaces":8,"volumes":8},"univ_indices":7,"univ_types":7,"volume_ids":24,"volume_instance_ids":24,"volume_records":24}})json",
R"json({"_category":"internal","_label":"orange","bih_metadata":{"max_depth":[5,0,0,4,2,0,0],"num_finite_bboxes":[6,0,0,4,2,0,0],"num_nonfinite_bboxes":[1,0,0,0,0,0,0]},"scalars":{"logic":"postfix","max_csg_levels":1,"max_faces":8,"max_intersections":14,"num_univ_levels":3,"tol":{"abs":1e-05,"rel":1e-05}},"sizes":{"bih":{"bboxes":24,"inner_nodes":9,"leaf_nodes":16,"local_volume_ids":13},"connectivity_records":13,"daughters":6,"fast_real3s":0,"local_surface_ids":20,"local_volume_ids":18,"logic_ints":31,"obz_records":0,"real_ids":13,"reals":46,"rect_arrays":0,"simple_units":7,"surface_types":13,"transforms":6,"univ_indices":7,"univ_types":7,"universe_indexer":{"surfaces":8,"volumes":8},"volume_ids":24,"volume_instance_ids":24,"volume_records":24}})json",
Copy link
Member

Choose a reason for hiding this comment

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

Also not ideal is that there are so many trees with depth == num_finite...

Note that the input file is built by UnitProto.test.cc, and when you run that, it spits out inputbuilder-hierarchy.csg.json, which contains all the bzones and volumes for each universe. That lets us see that the no-bbox cases are for "leaf" nodes that have only a single background volume and no interior volumes.

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