Skip to content

Add options for constructing an ORANGE geometry #2045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
elliottbiondo wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft

Add options for constructing an ORANGE geometry #2045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4bdcf5c
Change BIH::Input to BIH::Options
elliottbiondo Oct 13, 2025
0ddc337
Construct BIH based options in OrangeInput
elliottbiondo Oct 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion src/orange/OrangeInput.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
#include "surf/VariantSurface.hh"
#include "transform/VariantTransform.hh"

#include "detail/BIHBuilder.hh"

namespace celeritas
{
//---------------------------------------------------------------------------//
Expand Down Expand Up @@ -159,6 +161,19 @@ struct RectArrayInput
}
};

//---------------------------------------------------------------------------//
/*!
* Options that govern the geometry construction process.
*/
struct ConstructionOptions
{
//! Options for Bounding Interval Hierarchy (BIH) construction
detail::BIHBuilder::Options bih_options;
Copy link
Member

@sethrj sethrj Oct 16, 2025

Choose a reason for hiding this comment

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

It might make it more messy in the short term, but I think the preferred pattern will look more like the newer celeritas code: inp/Foo.hh has the setup options for Foo. So we'd have orange/inp/Bih.hh which would have struct inp::Bih and that would replace BihBuilder::Options. We'd likewise move orange/gorg/Options.hh to orange/inp/Converter.hh. Finally we'd also move OrangeInput to orange/inp/Model.hh or similar.

Copy link
Contributor Author

@elliottbiondo elliottbiondo Oct 16, 2025

Choose a reason for hiding this comment

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

FWIW, I think Options makes more sense, lest we overload the word "input"; we already have an orangeinp folder. Likewise, below, I think semantically the input to the BIHBuilder are the bounding boxes, whereas the min_split_size is an option specifying how the process is done.

Copy link
Contributor Author

@elliottbiondo elliottbiondo Oct 16, 2025

Choose a reason for hiding this comment

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

The orange/inp folder doesn't currently exist. I can make, but I am still not clear on how InputBuilder::Options fits in (seems like that should be defined at the OrangeInput level).

I also, I think g4org::Options should consist of: 1) a high-level struct will all general OrangeOptions, that is also used for a non-G4 workflow to specify all ORANGE options in one place, and 2) additional G4-specific options.

Copy link
Member

@sethrj sethrj Oct 16, 2025

Choose a reason for hiding this comment

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

The weirdness/confusion is because we've been inconsistent in the past and are transitioning toward having an inp namespace that has all of the input structs that were formerly called Input/Options. This namespace is to be the primary interface with the outside parts of the code: see #2029.

In a hypothetical/future refactorization that I am talking about:

  • celeritas::OrangeInput -> celeritas::inp::Orange
  • celeritas::detail::BIHBuilder::Options -> celeritas::inp::BihBuilder
  • celeritas::g4org::Options -> celeritas::inp::Converter (?)
  • celeritas::orangeinp -> celeritas::setup (?)
  • celeritas::g4org -> celeritas::setup::g4org (?)

I don't know what's the right path between that future and our present, but I would at least like:

  • BIH builder doesn't include half of orange transitively via the OrangeInput (since VariantSurfaces)
  • OrangeInput doesn't include and rely on a detail namespace
  • Mlutiple layers of nested options with only one member per struct


//! Whether the options are valid
explicit operator bool() const { return static_cast<bool>(bih_options); }
};

//---------------------------------------------------------------------------//
//! Possible types of universe inputs
using VariantUniverseInput = std::variant<UnitInput, RectArrayInput>;
Expand All @@ -171,11 +186,18 @@ struct OrangeInput
{
std::vector<VariantUniverseInput> universes;

ConstructionOptions construction_opts;

//! Relative and absolute error for construction and transport
Tolerance<> tol;

//! Whether the unit definition is valid
explicit operator bool() const { return !universes.empty() && tol; }
explicit operator bool() const
{
return !universes.empty() &&

static_cast<bool>(construction_opts) && tol;
}
};

//---------------------------------------------------------------------------//
Expand Down
3 changes: 2 additions & 1 deletion src/orange/OrangeParams.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,8 @@ OrangeParams::OrangeParams(OrangeInput&& input, SPConstVolumes&& volumes)
&impl_volume_labels,
&host_data};
Overload insert_universe{
detail::UnitInserter{&insert_universe_base, &host_data},
detail::UnitInserter{
&insert_universe_base, &host_data, &(input.construction_opts)},
detail::RectArrayInserter{&insert_universe_base, &host_data}};

for (auto&& u : input.universes)
Expand Down
10 changes: 5 additions & 5 deletions src/orange/detail/BIHBuilder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@ namespace detail
{
//---------------------------------------------------------------------------//
/*!
* Construct from Storage and Input objects.
* Construct from Storage and Options objects.
*/
BIHBuilder::BIHBuilder(Storage* storage, Input inp)
BIHBuilder::BIHBuilder(Storage* storage, Options options)
: bboxes_{&storage->bboxes}
, local_volume_ids_{&storage->local_volume_ids}
, inner_nodes_{&storage->inner_nodes}
, leaf_nodes_{&storage->leaf_nodes}
, inp_{inp}
, options_{options}
{
CELER_EXPECT(storage);
CELER_EXPECT(inp_.min_split_size > 1);
CELER_EXPECT(options_.min_split_size > 1);
}

//---------------------------------------------------------------------------//
Expand Down Expand Up @@ -138,7 +138,7 @@ void BIHBuilder::construct_tree(VecIndices const& indices,
(*nodes)[current_index] = node;
};

if (indices.size() < inp_.min_split_size)
if (indices.size() < options_.min_split_size)
{
// All bboxes fit on a single leaf; make it and exit early
make_leaf();
Expand Down
15 changes: 9 additions & 6 deletions src/orange/detail/BIHBuilder.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,14 @@ class BIHBuilder
//!@{
//! \name Type aliases

//! Input parameters
struct Input
//! Options parameters
struct Options
Copy link
Member

@sethrj sethrj Oct 16, 2025

Choose a reason for hiding this comment

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

if we move the struct, this'll become using Input = inp::BihBuilder; as is done in the other update celeritas classes.

{
//! Minimum number of bboxes needed to trigger a partitioning attempt
size_type min_split_size;
size_type min_split_size = 2;

//! Whether the options are valid
explicit operator bool() const { return min_split_size >= 2; }
};

using VecBBox = std::vector<FastBBox>;
Expand All @@ -64,8 +67,8 @@ class BIHBuilder
//!@}

public:
// Construct from Storage and Input objects
BIHBuilder(Storage* storage, Input inp);
// Construct from Storage and Options objects
BIHBuilder(Storage* storage, Options options);

// Create BIH Nodes
BIHTree operator()(VecBBox&& bboxes, SetLocalVolId const& implicit_vol_id);
Expand Down Expand Up @@ -94,7 +97,7 @@ class BIHBuilder
CollectionBuilder<BIHInnerNode> inner_nodes_;
CollectionBuilder<BIHLeafNode> leaf_nodes_;

Input inp_;
Options options_;

//// HELPER FUNCTIONS ////

Expand Down
6 changes: 4 additions & 2 deletions src/orange/detail/UnitInserter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,11 @@ std::string to_string(VolumeInput::VariantLabel const& vlabel)
/*!
* Construct from full parameter data.
*/
UnitInserter::UnitInserter(UniverseInserter* insert_universe, Data* orange_data)
UnitInserter::UnitInserter(UniverseInserter* insert_universe,
Data* orange_data,
ConstructionOptions* opts)
: orange_data_(orange_data)
, build_bih_tree_{&orange_data_->bih_tree_data, BIHBuilder::Input{2}}
, build_bih_tree_{&orange_data_->bih_tree_data, opts->bih_options}
, insert_transform_{&orange_data_->transforms, &orange_data_->reals}
, build_surfaces_{&orange_data_->surface_types,
&orange_data_->real_ids,
Expand Down
4 changes: 3 additions & 1 deletion src/orange/detail/UnitInserter.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ class UnitInserter

public:
// Construct from full parameter data
UnitInserter(UniverseInserter* insert_universe, Data* orange_data);
UnitInserter(UniverseInserter* insert_universe,
Data* orange_data,
ConstructionOptions* opts);

// Create a simple unit and store in in OrangeParamsData
UniverseId operator()(UnitInput&& inp);
Expand Down
16 changes: 8 additions & 8 deletions test/orange/detail/BIHBuilder.test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ TEST_F(BIHBuilderTest, basic)
{{0, -1, 0}, {5, 0, 100}},
};

BIHBuilder build(&storage_, BIHBuilder::Input{2});
BIHBuilder build(&storage_, BIHBuilder::Options{2});
auto bih_tree = build(std::move(bboxes), implicit_vol_ids_);

ASSERT_EQ(1, bih_tree.inf_vol_ids.size());
Expand Down Expand Up @@ -278,7 +278,7 @@ TEST_F(BIHBuilderTest, grid)
}
}

BIHBuilder build(&storage_, BIHBuilder::Input{2});
BIHBuilder build(&storage_, BIHBuilder::Options{2});
auto bih_tree = build(std::move(bboxes), implicit_vol_ids_);
ASSERT_EQ(1, bih_tree.inf_vol_ids.size());
EXPECT_EQ(LocalVolumeId{0},
Expand Down Expand Up @@ -534,7 +534,7 @@ TEST_F(BIHBuilderTest, grid_less_split)
}
}

BIHBuilder build(&storage_, BIHBuilder::Input{5});
BIHBuilder build(&storage_, BIHBuilder::Options{5});
auto bih_tree = build(std::move(bboxes), implicit_vol_ids_);
ASSERT_EQ(1, bih_tree.inf_vol_ids.size());
EXPECT_EQ(LocalVolumeId{0},
Expand Down Expand Up @@ -667,7 +667,7 @@ TEST_F(BIHBuilderTest, single_finite_volume)
{
VecFastBbox bboxes = {{{0, 0, 0}, {1, 1, 1}}};

BIHBuilder build(&storage_, BIHBuilder::Input{2});
BIHBuilder build(&storage_, BIHBuilder::Options{2});
auto bih_tree = build(std::move(bboxes), implicit_vol_ids_);

ASSERT_EQ(0, bih_tree.inf_vol_ids.size());
Expand All @@ -687,7 +687,7 @@ TEST_F(BIHBuilderTest, multiple_nonpartitionable_volumes)
{{0, 0, 0}, {1, 1, 1}},
};

BIHBuilder build(&storage_, BIHBuilder::Input{2});
BIHBuilder build(&storage_, BIHBuilder::Options{2});
auto bih_tree = build(std::move(bboxes), implicit_vol_ids_);

ASSERT_EQ(0, bih_tree.inf_vol_ids.size());
Expand All @@ -705,7 +705,7 @@ TEST_F(BIHBuilderTest, single_infinite_volume)
{
VecFastBbox bboxes = {FastBBox::from_infinite()};

BIHBuilder build(&storage_, BIHBuilder::Input{2});
BIHBuilder build(&storage_, BIHBuilder::Options{2});
auto bih_tree = build(std::move(bboxes), implicit_vol_ids_);

ASSERT_EQ(0, bih_tree.inner_nodes.size());
Expand All @@ -723,7 +723,7 @@ TEST_F(BIHBuilderTest, multiple_infinite_volumes)
FastBBox::from_infinite(),
};

BIHBuilder build(&storage_, BIHBuilder::Input{2});
BIHBuilder build(&storage_, BIHBuilder::Options{2});
auto bih_tree = build(std::move(bboxes), implicit_vol_ids_);

ASSERT_EQ(0, bih_tree.inner_nodes.size());
Expand All @@ -749,7 +749,7 @@ TEST_F(BIHBuilderTest, TEST_IF_CELERITAS_DEBUG(semi_finite_volumes))
{{-inff, 0, 0}, {inff, 1, 1}},
};

BIHBuilder build(&storage_, BIHBuilder::Input{2});
BIHBuilder build(&storage_, BIHBuilder::Options{2});
EXPECT_THROW(build(std::move(bboxes), implicit_vol_ids_), DebugError);
}

Expand Down
12 changes: 6 additions & 6 deletions test/orange/detail/BIHEnclosingVolFinder.test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ TEST_F(BIHEnclosingVolFinderTest, basic)
{{0, -1, 0}, {5, 0, 100}},
};

BIHBuilder build(&storage_, BIHBuilder::Input{min_split_size});
BIHBuilder build(&storage_, BIHBuilder::Options{min_split_size});
auto bih_tree = build(std::move(bboxes), implicit_vol_ids_);

ref_storage_ = storage_;
Expand Down Expand Up @@ -128,7 +128,7 @@ TEST_F(BIHEnclosingVolFinderTest, grid)
}
}

BIHBuilder build(&storage_, BIHBuilder::Input{min_split_size});
BIHBuilder build(&storage_, BIHBuilder::Options{min_split_size});
auto bih_tree = build(std::move(bboxes), implicit_vol_ids_);

ref_storage_ = storage_;
Expand Down Expand Up @@ -163,7 +163,7 @@ TEST_F(BIHEnclosingVolFinderTest, single_finite_volume)
{
VecFastBbox bboxes = {{{0, 0, 0}, {1, 1, 1}}};

BIHBuilder build(&storage_, BIHBuilder::Input{2});
BIHBuilder build(&storage_, BIHBuilder::Options{2});
auto bih_tree = build(std::move(bboxes), implicit_vol_ids_);

ref_storage_ = storage_;
Expand All @@ -179,7 +179,7 @@ TEST_F(BIHEnclosingVolFinderTest, multiple_nonpartitionable_volumes)
{{0, 0, 0}, {1, 1, 1}},
};

BIHBuilder build(&storage_, BIHBuilder::Input{2});
BIHBuilder build(&storage_, BIHBuilder::Options{2});
auto bih_tree = build(std::move(bboxes), implicit_vol_ids_);

ref_storage_ = storage_;
Expand All @@ -193,7 +193,7 @@ TEST_F(BIHEnclosingVolFinderTest, single_infinite_volume)
{
VecFastBbox bboxes = {FastBBox::from_infinite()};

BIHBuilder build(&storage_, BIHBuilder::Input{2});
BIHBuilder build(&storage_, BIHBuilder::Options{2});
auto bih_tree = build(std::move(bboxes), implicit_vol_ids_);

ref_storage_ = storage_;
Expand All @@ -209,7 +209,7 @@ TEST_F(BIHEnclosingVolFinderTest, multiple_infinite_volumes)
FastBBox::from_infinite(),
};

BIHBuilder build(&storage_, BIHBuilder::Input{2});
BIHBuilder build(&storage_, BIHBuilder::Options{2});
auto bih_tree = build(std::move(bboxes), implicit_vol_ids_);

ref_storage_ = storage_;
Expand Down
2 changes: 1 addition & 1 deletion test/orange/detail/BIHIntersectingVolFinder.test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class BIHIntersectingVolFinderTest : public Test
{{0, -1, 0}, {5, 0, 100}},
};

BIHBuilder build(&storage_, BIHBuilder::Input{min_split_size});
BIHBuilder build(&storage_, BIHBuilder::Options{min_split_size});
BIHBuilder::SetLocalVolId implicit_vol_ids_;
bih_tree_ = build(std::move(bboxes), implicit_vol_ids_);
ref_storage_ = storage_;
Expand Down
Loading