From 167896a0f6570d4835a1a73c481556c8883fc991 Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Thu, 5 Feb 2026 09:52:52 -0500 Subject: [PATCH 01/24] Add bzone stream --- src/orange/orangeinp/detail/BoundingZone.cc | 36 +++++++++++++++++++++ src/orange/orangeinp/detail/BoundingZone.hh | 5 +++ 2 files changed, 41 insertions(+) diff --git a/src/orange/orangeinp/detail/BoundingZone.cc b/src/orange/orangeinp/detail/BoundingZone.cc index 60423ce596..32e4394e88 100644 --- a/src/orange/orangeinp/detail/BoundingZone.cc +++ b/src/orange/orangeinp/detail/BoundingZone.cc @@ -209,6 +209,42 @@ BBox get_exterior_bbox(BoundingZone const& bz) return bz.exterior; } +//---------------------------------------------------------------------------// +/*! + * Print for debugging. + */ +std::ostream& operator<<(std::ostream& os, BoundingZone const& bz) +{ + os << "{"; + char const* comma = ""; + if (!bz.negated) + { + if (bz.interior) + { + os << "encloses " << bz.interior; + comma = ", "; + } + if (!is_infinite(bz.exterior)) + { + os << comma << "enclosed by " << bz.exterior; + } + } + else + { + if (!is_infinite(bz.exterior)) + { + os << "encloses " << bz.exterior; + comma = ", "; + } + if (bz.interior) + { + os << comma << "excluded from " << bz.interior; + } + } + os << '}'; + return os; +} + //---------------------------------------------------------------------------// } // namespace detail } // namespace orangeinp diff --git a/src/orange/orangeinp/detail/BoundingZone.hh b/src/orange/orangeinp/detail/BoundingZone.hh index 06b98cf894..957d28d9e5 100644 --- a/src/orange/orangeinp/detail/BoundingZone.hh +++ b/src/orange/orangeinp/detail/BoundingZone.hh @@ -6,6 +6,8 @@ //---------------------------------------------------------------------------// #pragma once +#include + #include "geocel/BoundingBox.hh" #include "orange/BoundingBoxUtils.hh" @@ -117,6 +119,9 @@ BoundingZone calc_union(BoundingZone const& a, BoundingZone const& b); // Get an infinite bbox if "negated", else get the exterior BBox get_exterior_bbox(BoundingZone const&); +// Print for debugging +std::ostream& operator<<(std::ostream& os, BoundingZone const& bz); + //---------------------------------------------------------------------------// /*! * Flip inside and outside. From 86a29ad341704e63b5badb9ea27ed09d9c9af93a Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Thu, 5 Feb 2026 09:54:45 -0500 Subject: [PATCH 02/24] Add additional debug/warning --- src/orange/orangeinp/UnitProto.cc | 120 ++++++++++++++++++------------ 1 file changed, 74 insertions(+), 46 deletions(-) diff --git a/src/orange/orangeinp/UnitProto.cc b/src/orange/orangeinp/UnitProto.cc index d1cbdd29a4..299353153b 100644 --- a/src/orange/orangeinp/UnitProto.cc +++ b/src/orange/orangeinp/UnitProto.cc @@ -101,57 +101,76 @@ void remove_negated_join(CsgUnit& unit, std::string_view label) std::map regions; // Map metadata - for (auto old_id : range(id_cast(unit.tree.size()))) + for (auto old_id : range(NodeId{unit.tree.size()})) { auto& old_md = unit.metadata[old_id.get()]; - if (auto new_id = nodes[old_id.get()]) - { - CELER_ASSERT(new_id < metadata.size()); - - // Update metadata - auto& new_md = metadata[new_id.get()]; - if (CELER_UNLIKELY(!new_md.empty())) + auto new_id = nodes[old_id.get()]; + auto log_debug = [&, msg = std::optional{}]() mutable + -> Logger::Message& { + if (!msg) { - // Node was merged with another node - CELER_LOG(warning) - << "Merged CSG node " - << join(new_md.begin(), new_md.end(), "','") << " into " - << join(old_md.begin(), old_md.end(), "','"); - new_md.insert(old_md.begin(), old_md.end()); - old_md.clear(); + msg.emplace(CELER_LOG(debug)); + *msg << "In universe '" << label << "': node " << old_id + << " maps to " << new_id << ": "; } else { - new_md = std::move(old_md); + *msg << "; "; + } + return *msg; + }; + + if (new_id) + { + CELER_ASSERT(new_id < metadata.size()); + if (true) + { + // Update metadata + auto& new_md = metadata[new_id.get()]; + if (CELER_UNLIKELY(!new_md.empty())) + { + // Node was merged with another node + log_debug() + << "merged '" + << join(new_md.begin(), new_md.end(), "','") + << "' into '" + << join(old_md.begin(), old_md.end(), "','") << "'"; + new_md.insert(old_md.begin(), old_md.end()); + old_md.clear(); + } + else + { + new_md = std::move(old_md); + } } - // Update region + // Move region to new tree with updated ID: necessary even for + // false/true region due to interior if (auto iter = unit.regions.find(old_id); iter != unit.regions.end()) { auto region = unit.regions.extract(iter); region.key() = new_id; - regions.insert(std::move(region)); + auto irt = regions.insert(std::move(region)); + if (!irt.inserted) + { + log_debug() << "merged region bounds"; + } } } else { // Node was removed from the tree auto region = unit.regions.find(old_id); - if (CELER_UNLIKELY(region != unit.regions.end() || !old_md.empty())) + if (region != unit.regions.end()) { - auto msg = CELER_LOG(warning); - msg << "Simplification removed node " << old_id.get(); - if (!old_md.empty()) - { - msg << "='" << join(old_md.begin(), old_md.end(), "','") - << "'"; - } - if (region != unit.regions.end()) - { - msg << " (which has a region)"; - } - msg << " from '" << label << "'"; + log_debug() << "deleted region"; + } + if (!old_md.empty()) + { + log_debug() + << "deleted '" << join(old_md.begin(), old_md.end(), "','") + << "'"; } } } @@ -266,6 +285,12 @@ void UnitProto::build(ProtoBuilder& pb) const { result.surfaces.emplace_back(csg_unit.surfaces[lsid.get()]); } + auto find_new_lsid = [&sls = sorted_local_surfaces](LocalSurfaceId old) { + CELER_EXPECT(old); + auto iter = find_sorted(sls.begin(), sls.end(), old); + CELER_ASSERT(iter != sls.end()); + return id_cast(iter - sls.begin()); + }; // Save surface labels result.surface_labels.resize(result.surfaces.size()); @@ -273,21 +298,24 @@ void UnitProto::build(ProtoBuilder& pb) const { if (auto* surf_node = std::get_if(&csg_unit.tree[node_id])) { - LocalSurfaceId old_lsid = surf_node->id; - auto idx = static_cast( - find_sorted(sorted_local_surfaces.begin(), - sorted_local_surfaces.end(), - old_lsid) - - sorted_local_surfaces.begin()); - CELER_ASSERT(idx < result.surface_labels.size()); - - // NOTE: surfaces may be created more than once. Our primitive - // "input" allows association with only one surface, so we'll - // arbitrarily choose the lexicographically sorted "first" surface - // name in the list. - CELER_ASSERT(!csg_unit.metadata[node_id.get()].empty()); - auto const& label = *csg_unit.metadata[node_id.get()].begin(); - result.surface_labels[idx] = label; + auto new_lsid = find_new_lsid(surf_node->id); + CELER_ASSERT(new_lsid < result.surface_labels.size()); + + auto const& md = csg_unit.metadata[node_id.get()]; + if (auto iter = md.begin(); iter != md.end()) + { + // NOTE: surfaces may be created more than once. Our primitive + // "input" allows association with only one surface, so we'll + // arbitrarily choose the lexicographically sorted "first" + // surface name in the list. + result.surface_labels[new_lsid.get()] = *iter; + } + else + { + CELER_LOG(warning) << "No metadata for new surface " + << new_lsid << " (new node ID " << node_id + << ") = old LSID " << surf_node->id; + } } } From de97c14940736c72195d4f8a830b490abb712bb8 Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Thu, 5 Feb 2026 10:17:04 -0500 Subject: [PATCH 03/24] Add test --- test/orange/g4org/Converter.test.cc | 2 +- test/orange/g4org/ProtoConstructor.test.cc | 39 ++++++++++++++++++---- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/test/orange/g4org/Converter.test.cc b/test/orange/g4org/Converter.test.cc index 53b4b3f093..97865544a6 100644 --- a/test/orange/g4org/Converter.test.cc +++ b/test/orange/g4org/Converter.test.cc @@ -351,7 +351,7 @@ TEST_F(ConverterTest, znenv) auto result = convert(this->geo(), *this->volumes()).input; write_org_json(result, basename); - EXPECT_EQ(9, result.universes.size()); + ASSERT_EQ(9, result.universes.size()); { auto const& unit = std::get(result.universes[0]); EXPECT_EQ(6, unit.volumes.size()); diff --git a/test/orange/g4org/ProtoConstructor.test.cc b/test/orange/g4org/ProtoConstructor.test.cc index 83c5676d9e..a07f15774f 100644 --- a/test/orange/g4org/ProtoConstructor.test.cc +++ b/test/orange/g4org/ProtoConstructor.test.cc @@ -312,7 +312,7 @@ TEST_F(LarSphereTest, default) } } -//----------------------------m-----------------------------------------------// +//----------------------------------------------------------------------------// using MultilevelTest = ProtoConstructorTest; TEST_F(MultilevelTest, full_inline) @@ -692,18 +692,45 @@ TEST_F(ZnenvTest, default) EXPECT_EQ(GeoMatId{}, u.background); } { + // Lots of eliminations happen in ZNTX SCOPED_TRACE("ZNTX"); - auto u = this->build_unit(protos, UnivId{1}); - static char const* const expected_surface_strings[] = { - "Plane: y=0", - }; + static char const* const expected_surface_strings[] = {"Plane: y=0"}; static char const* const expected_volume_strings[] = {"F", "-6", "+6", "F"}; + static char const* const expected_md_strings[] = { + R"(TGeoBBox0x0,TGeoBBox0x0@mx,TGeoBBox0x0@my,TGeoBBox0x0@mz,TGeoBBox0x0@mx,TGeoBBox0x0@my,TGeoBBox0x0@mz,ZNTX.children)", + R"(TGeoBBox0x0@px,TGeoBBox0x0@py,TGeoBBox0x0@pz,TGeoBBox0x0@px,TGeoBBox0x0@py,TGeoBBox0x0@pz,ZNTX,[EXTERIOR])", + "TGeoBBox0x0,TGeoBBox0x0@my,TGeoBBox0x0@py", + "TGeoBBox0x0", + }; + static char const* const expected_bound_strings[] = { + R"(0: {{{-1.76,-3.52,-50}, {1.76,3.52,50}}, {{-1.76,-3.52,-50}, {1.76,3.52,50}}})", + R"(~1: {{{-1.76,-3.52,-50}, {1.76,3.52,50}}, {{-1.76,-3.52,-50}, {1.76,3.52,50}}})", + R"(2: {{{-1.76,0,-50}, {1.76,3.52,50}}, {{-1.76,0,-50}, {1.76,3.52,50}}})", + R"(3: {{{-1.76,-3.52,-50}, {1.76,0,50}}, {{-1.76,-3.52,-50}, {1.76,0,50}}})"}; + static char const* const expected_trans_strings[] = { + "0: t=0 -> {}", + "1: t=0", + "2: t=2 -> {{0,1.76,0}}", + "3: t=1 -> {{0,-1.76,0}}", + }; + static char const* const expected_fill_strings[] + = {"", "{u=0, t=1}", "{u=1, t=2}", "m2"}; + static int const expected_volume_nodes[] = {1, 3, 2, 1}; + static char const expected_tree_string[] + = R"json(["t",["~",0],["S",6],["~",2]])json"; + + auto u = this->build_unit(protos, UnivId{1}); EXPECT_VEC_EQ(expected_surface_strings, surface_strings(u)); EXPECT_VEC_EQ(expected_volume_strings, volume_strings(u)); - EXPECT_EQ(GeoMatId{}, u.background); + EXPECT_VEC_EQ(expected_md_strings, md_strings(u)); + EXPECT_VEC_EQ(expected_bound_strings, bound_strings(u)); + EXPECT_VEC_EQ(expected_trans_strings, transform_strings(u)); + EXPECT_VEC_EQ(expected_fill_strings, fill_strings(u)); + EXPECT_VEC_EQ(expected_volume_nodes, volume_nodes(u)); + EXPECT_JSON_EQ(expected_tree_string, tree_string(u)); } { SCOPED_TRACE("ZN1"); From 40e2c8445b5d0c194a24e0a196761d5249ed7ad8 Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Thu, 5 Feb 2026 12:44:06 -0500 Subject: [PATCH 04/24] Replace constexpr function with inline constant --- src/orange/OrangeData.hh | 5 +---- src/orange/OrangeParams.cc | 2 +- src/orange/detail/UnitInserter.cc | 2 +- src/orange/orangeinp/UnitProto.cc | 2 +- src/orange/orangeinp/UnitProto.hh | 3 +-- src/orange/univ/SimpleUnitTracker.hh | 2 +- 6 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/orange/OrangeData.hh b/src/orange/OrangeData.hh index 4e01f1d7ae..fce16e1883 100644 --- a/src/orange/OrangeData.hh +++ b/src/orange/OrangeData.hh @@ -38,10 +38,7 @@ inline constexpr UnivId orange_global_univ{0}; inline constexpr UnivLevelId orange_global_univ_level{0}; //! Logic notation used for boolean expressions -CELER_FUNCTION inline constexpr auto orange_tracking_logic() -{ - return LogicNotation::infix; -} +inline constexpr auto orange_tracking_logic{LogicNotation::infix}; //---------------------------------------------------------------------------// /*! diff --git a/src/orange/OrangeParams.cc b/src/orange/OrangeParams.cc index 734ee70cc2..6c04b35d9d 100644 --- a/src/orange/OrangeParams.cc +++ b/src/orange/OrangeParams.cc @@ -197,7 +197,7 @@ OrangeParams::OrangeParams(OrangeInput&& input, SPConstVolumes&& volumes) host_data.scalars.num_vol_levels = volumes_ ? volumes_->num_volume_levels() : 0; - detail::convert_logic(input, orange_tracking_logic()); + detail::convert_logic(input, orange_tracking_logic); // Insert all universes { diff --git a/src/orange/detail/UnitInserter.cc b/src/orange/detail/UnitInserter.cc index b0f11821a7..bed068989a 100644 --- a/src/orange/detail/UnitInserter.cc +++ b/src/orange/detail/UnitInserter.cc @@ -578,7 +578,7 @@ VolumeRecord UnitInserter::insert_volume(SurfacesRecord const& surf_record, } static auto const nowhere_logic = []() -> std::array { - if (orange_tracking_logic() == LogicNotation::postfix) + if (orange_tracking_logic == LogicNotation::postfix) return {logic::ltrue, logic::lnot}; else return {logic::lnot, logic::ltrue}; diff --git a/src/orange/orangeinp/UnitProto.cc b/src/orange/orangeinp/UnitProto.cc index 299353153b..ad916c5cc0 100644 --- a/src/orange/orangeinp/UnitProto.cc +++ b/src/orange/orangeinp/UnitProto.cc @@ -627,7 +627,7 @@ auto UnitProto::build(Tol const& tol, remove_interior(result, this->label()); } - if (orange_tracking_logic() == LogicNotation::infix + if (orange_tracking_logic == LogicNotation::infix || input_.remove_negated_join) { remove_negated_join(result, this->label()); diff --git a/src/orange/orangeinp/UnitProto.hh b/src/orange/orangeinp/UnitProto.hh index c9d988c367..3f655f6802 100644 --- a/src/orange/orangeinp/UnitProto.hh +++ b/src/orange/orangeinp/UnitProto.hh @@ -135,8 +135,7 @@ class UnitProto : public ProtoInterface //! For non-global units, assume inside the boundary bool remove_interior{true}; //! Use DeMorgan's law to remove negated joins - bool remove_negated_join{orange_tracking_logic() - == LogicNotation::infix}; + bool remove_negated_join{orange_tracking_logic == LogicNotation::infix}; //!@} diff --git a/src/orange/univ/SimpleUnitTracker.hh b/src/orange/univ/SimpleUnitTracker.hh index 6f553cf5c5..0d7d47079d 100644 --- a/src/orange/univ/SimpleUnitTracker.hh +++ b/src/orange/univ/SimpleUnitTracker.hh @@ -53,7 +53,7 @@ class SimpleUnitTracker using Intersection = detail::Intersection; using LocalState = detail::LocalState; using LogicEvaluator - = std::conditional_t; //!@} From fe587df231e86e22695bf8b1ffaf56226df778c7 Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Thu, 5 Feb 2026 15:52:53 -0500 Subject: [PATCH 05/24] Remove duplicate function declaration --- src/orange/detail/OrangeInputIOImpl.json.hh | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/orange/detail/OrangeInputIOImpl.json.hh b/src/orange/detail/OrangeInputIOImpl.json.hh index bf265da4d1..1a3300ed97 100644 --- a/src/orange/detail/OrangeInputIOImpl.json.hh +++ b/src/orange/detail/OrangeInputIOImpl.json.hh @@ -3,14 +3,13 @@ // SPDX-License-Identifier: (Apache-2.0 OR MIT) //---------------------------------------------------------------------------// //! \file orange/detail/OrangeInputIOImpl.json.hh +//! \sa LogicUtils.hh //---------------------------------------------------------------------------// #pragma once -#include #include #include -#include "orange/OrangeTypes.hh" #include "orange/surf/VariantSurface.hh" #include "orange/transform/VariantTransform.hh" @@ -31,12 +30,6 @@ std::vector import_zipped_surfaces(nlohmann::json const& j); // Write surface data to a JSON object nlohmann::json export_zipped_surfaces(std::vector const& s); -// Build a logic definition from a C string. -std::vector string_to_logic(std::string const& s); - -// Convert a logic vector to a string -std::string logic_to_string(std::vector const&); - //---------------------------------------------------------------------------// } // namespace detail } // namespace celeritas From 2bcba713279f8e1d90c4bdff7631dfd57bd73380 Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Thu, 5 Feb 2026 16:03:00 -0500 Subject: [PATCH 06/24] Extend string parsing and remove lend --- src/orange/OrangeTypes.hh | 10 ++-- src/orange/detail/LogicUtils.cc | 21 ++++++-- src/orange/detail/LogicUtils.hh | 6 ++- test/orange/detail/LogicUtils.test.cc | 9 +--- .../orange/univ/detail/InfixEvaluator.test.cc | 48 ++++------------- .../orange/univ/detail/LogicEvaluator.test.cc | 53 +++++-------------- 6 files changed, 50 insertions(+), 97 deletions(-) diff --git a/src/orange/OrangeTypes.hh b/src/orange/OrangeTypes.hh index e43efd390f..759102b043 100644 --- a/src/orange/OrangeTypes.hh +++ b/src/orange/OrangeTypes.hh @@ -251,18 +251,20 @@ enum class Chirality : bool */ namespace logic { -//! Special logical Evaluator tokens ordered by precedence. -// The enum values are set to the highest 6 values of logic_int. +/*! + * Special logical evaluator tokens ordered by precedence. + * + * The enum values are set to the highest 6 values of logic_int. + */ enum OperatorToken : logic_int { - lbegin = logic_int(~logic_int(6)), + lbegin = static_cast(-6), lopen = lbegin, //!< Open parenthesis lclose, //!< Close parenthesis lor, //!< Binary logical OR land, //!< Binary logical AND lnot, //!< Unary negation ltrue, //!< Push 'true' - lend }; } // namespace logic diff --git a/src/orange/detail/LogicUtils.cc b/src/orange/detail/LogicUtils.cc index 0c413c772e..eb2c6821d0 100644 --- a/src/orange/detail/LogicUtils.cc +++ b/src/orange/detail/LogicUtils.cc @@ -418,7 +418,7 @@ std::vector convert_to_postfix(Span infix) /*! * Build a logic definition from a C string. * - * A valid string satisfies the regex "[0-9~!| ]+", but the result may + * A valid string satisfies the regex "[0-9~!| ()]+", but the result may * not be a valid logic expression. (The volume inserter will ensure that the * logic expression at least is consistent for a CSG region definition.) * @@ -429,12 +429,13 @@ std::vector convert_to_postfix(Span infix) \endcode */ -std::vector string_to_logic(std::string const& s) +VecLogic string_to_logic(std::string_view s) { - std::vector result; + VecLogic result; logic_int surf_id{}; bool reading_surf{false}; + int parens_depth{0}; for (char v : s) { if (v >= '0' && v <= '9') @@ -460,11 +461,21 @@ std::vector string_to_logic(std::string const& s) // NOLINTNEXTLINE(bugprone-switch-missing-default-case) switch (v) { + case '(': + result.push_back(logic::lopen); + ++parens_depth; + continue; + case ')': + CELER_VALIDATE(parens_depth > 0, + << "unmatched ')' in logic string"); + --parens_depth; + result.push_back(logic::lclose); + continue; // clang-format off - case '*': result.push_back(logic::ltrue); continue; case '|': result.push_back(logic::lor); continue; case '&': result.push_back(logic::land); continue; case '~': result.push_back(logic::lnot); continue; + case '*': result.push_back(logic::ltrue); continue; // clang-format on } CELER_VALIDATE(v == ' ', @@ -476,6 +487,8 @@ std::vector string_to_logic(std::string const& s) result.push_back(surf_id); } + CELER_VALIDATE(parens_depth == 0, << "unmatched '(' in logic string"); + return result; } diff --git a/src/orange/detail/LogicUtils.hh b/src/orange/detail/LogicUtils.hh index 65a4147690..60db6f453d 100644 --- a/src/orange/detail/LogicUtils.hh +++ b/src/orange/detail/LogicUtils.hh @@ -7,6 +7,7 @@ #pragma once #include +#include #include #include "corecel/Assert.hh" @@ -20,6 +21,9 @@ namespace celeritas { namespace detail { +//---------------------------------------------------------------------------// +using VecLogic = std::vector; + //---------------------------------------------------------------------------// /*! * Convert a logic token to a string. @@ -62,7 +66,7 @@ std::vector convert_to_postfix(Span infix); /*! * Build a logic definition from a C string. */ -std::vector string_to_logic(std::string const& s); +std::vector string_to_logic(std::string_view s); //---------------------------------------------------------------------------// /*! diff --git a/test/orange/detail/LogicUtils.test.cc b/test/orange/detail/LogicUtils.test.cc index c27b786eb4..1566c2bc8e 100644 --- a/test/orange/detail/LogicUtils.test.cc +++ b/test/orange/detail/LogicUtils.test.cc @@ -94,14 +94,7 @@ TEST(NotationConverter, demorgan_postfix_to_infix) TEST(NotationConverter, demorgan_infix_to_infix) { - std::vector infix = { - logic::lnot, - logic::lopen, - logic_int{0}, - logic::lor, - logic_int{1}, - logic::lclose, - }; + std::vector infix = string_to_logic("~ ( 0 | 1 )"); auto input = make_input_with_logic(std::move(infix), LogicNotation::infix); convert_logic(input, LogicNotation::infix); auto& unit = std::get(input.universes.front()); diff --git a/test/orange/univ/detail/InfixEvaluator.test.cc b/test/orange/univ/detail/InfixEvaluator.test.cc index 35e5ed4c80..deb52a2e9b 100644 --- a/test/orange/univ/detail/InfixEvaluator.test.cc +++ b/test/orange/univ/detail/InfixEvaluator.test.cc @@ -8,6 +8,9 @@ #include +#include "orange/detail/LogicUtils.hh" +#include "orange/univ/detail/LogicStack.hh" + #include "celeritas_test.hh" namespace celeritas @@ -20,15 +23,6 @@ namespace test using VecSense = std::vector; -constexpr auto lbegin = logic::lbegin; -constexpr auto ltrue = logic::ltrue; -constexpr auto lor = logic::lor; -constexpr auto land = logic::land; -constexpr auto lnot = logic::lnot; -constexpr auto lopen = logic::lopen; -constexpr auto lclose = logic::lclose; -constexpr auto lend = logic::lend; - constexpr auto s_in = Sense::inside; constexpr auto s_out = Sense::outside; @@ -36,51 +30,27 @@ constexpr auto s_out = Sense::outside; // TESTS //---------------------------------------------------------------------------// -TEST(InfixEvaluatorTest, enumeration) -{ - EXPECT_GE(ltrue, lbegin); - EXPECT_GE(lnot, lbegin); - EXPECT_GE(land, lbegin); - EXPECT_GE(lor, lbegin); - EXPECT_GE(lopen, lbegin); - EXPECT_GE(lclose, lbegin); - EXPECT_LT(lbegin, lend); - - EXPECT_EQ('*', to_char(ltrue)); - EXPECT_EQ('|', to_char(lor)); - EXPECT_EQ('&', to_char(land)); - EXPECT_EQ('~', to_char(lnot)); - EXPECT_EQ('(', to_char(lopen)); - EXPECT_EQ(')', to_char(lclose)); -} - TEST(InfixEvaluatorTest, evaluate) { // Logic for alpha : !1 | 2 | !3 | 4 | !8 // With senses substituted: F | F | F | F | F - logic_int const alpha_logic[] - = {lnot, 1, lor, 2, lor, lnot, 3, lor, 4, lor, lnot, 8}; + auto const alpha_logic = string_to_logic("~1 | 2 |~3 | 4 | ~8"); // // Logic for beta : ((((5 & !1) & 6) & !7) & 8) // With senses substituted: ((((T & F) & F) & T) & T) - logic_int const beta_logic[] = { - lopen, lopen, lopen, lopen, 5, land, lnot, 1, lclose, land, - 6, lclose, land, lnot, 7, lclose, land, 8, lclose, - }; + auto const beta_logic = string_to_logic("((((5 & ~1) & 6) & ~7) & 8)"); // Logic for gamma : 8 ~ ~ ~ ~ // With senses substituted: T - logic_int const gamma_logic[] = {8}; + auto const gamma_logic = string_to_logic("8"); // Logic for delta : ((((!1 | 2 | !3 | 4) & !5 | 1 | !6 | 7) & 8) & !0) // With senses substituted: ((((F | F | F | T) & F | 1 | F | F) & T) & T) - logic_int const delta_logic[] = { - lopen, lopen, lopen, lopen, lnot, 1, lor, 2, lor, lnot, 3, - lor, 4, lclose, land, lnot, 5, lor, 1, lor, lnot, 6, - lor, 7, lclose, land, 8, lclose, land, lnot, 0, lclose}; + auto const delta_logic = string_to_logic( + "(((( ~1 | 2 | ~3 | 4) & ~5 | 1 | ~6 | 7) & 8) & ~0)"); - logic_int const everywhere_logic[] = {ltrue}; + auto const everywhere_logic = string_to_logic("*"); //// CREATE //// diff --git a/test/orange/univ/detail/LogicEvaluator.test.cc b/test/orange/univ/detail/LogicEvaluator.test.cc index 395660b3d3..28335706f8 100644 --- a/test/orange/univ/detail/LogicEvaluator.test.cc +++ b/test/orange/univ/detail/LogicEvaluator.test.cc @@ -6,7 +6,8 @@ //---------------------------------------------------------------------------// #include "orange/univ/detail/LogicEvaluator.hh" -#include +#include "orange/detail/LogicUtils.hh" +#include "orange/univ/detail/LogicStack.hh" #include "celeritas_test.hh" @@ -25,7 +26,6 @@ constexpr auto ltrue = logic::ltrue; constexpr auto lor = logic::lor; constexpr auto land = logic::land; constexpr auto lnot = logic::lnot; -constexpr auto lend = logic::lend; constexpr auto s_in = Sense::inside; constexpr auto s_out = Sense::outside; @@ -36,11 +36,13 @@ constexpr auto s_out = Sense::outside; TEST(LogicEvaluatorTest, enumeration) { + EXPECT_GE(logic::lclose, lbegin); + EXPECT_GE(logic::lopen, lbegin); EXPECT_GE(ltrue, lbegin); EXPECT_GE(lnot, lbegin); EXPECT_GE(land, lbegin); EXPECT_GE(lor, lbegin); - EXPECT_LT(lbegin, lend); + EXPECT_EQ(ltrue, static_cast(-1)); EXPECT_EQ('*', to_char(ltrue)); EXPECT_EQ('|', to_char(lor)); @@ -52,53 +54,22 @@ TEST(LogicEvaluatorTest, evaluate) { // Logic for alpha : 1 2 ~ & 3 & 4 ~ & ~ ~ 8 ~ ~ & ~ // With senses substituted: T F ~ & T & F ~ & T & ~ - logic_int const alpha_logic[] = {1, - 2, - lnot, - land, - 3, - land, - 4, - lnot, - land, - lnot, - lnot, - 8, - lnot, - lnot, - land, - lnot}; + auto const alpha_logic + = string_to_logic("1 2 ~ & 3 & 4 ~ & ~ ~ 8 ~ ~ & ~"); // Logic for beta : 5 1 ~ & 6 & 7 ~ & ~ ~ 8 ~ ~ & ~ // With senses substituted: T T ~ & F & F ~ & T & ~ - logic_int const beta_logic[] = {5, - 1, - lnot, - land, - 6, - land, - 7, - lnot, - land, - lnot, - lnot, - 8, - lnot, - lnot, - land, - lnot}; + auto const beta_logic = string_to_logic("5 1 ~ & 6 & 7 ~ & ~ ~ 8 ~ ~ & ~"); // Logic for gamma : 8 ~ ~ ~ ~ // With senses substituted: T - logic_int const gamma_logic[] = {8}; + auto const gamma_logic = string_to_logic("8"); // 1 2 ~ & 3 & 4 ~ & ~ 5 1 ~ & 6 & 7 ~ & ~ & 8 & 0 ~ & - logic_int const delta_logic[] = {1, 2, lnot, land, 3, land, 4, - lnot, land, lnot, 5, 1, lnot, land, - 6, land, 7, lnot, land, lnot, land, - 8, land, 0, lnot, land}; + auto const delta_logic = string_to_logic( + "1 2 ~ & 3 & 4 ~ & ~ 5 1 ~ & 6 & 7 ~ & ~ & 8 & 0 ~ &"); - logic_int const everywhere_logic[] = {ltrue}; + auto const everywhere_logic = string_to_logic("*"); //// CREATE //// From 56c1debc0a4a2a1f7de11391dd3c43269b10c61d Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Thu, 5 Feb 2026 16:49:03 -0500 Subject: [PATCH 07/24] Refactor CsgLogicUtils to use immutable policy factory pattern Split the builder pattern into two parts: - Policy classes: Immutable factories that can be passed by const reference - Visitor classes: Mutable workers that hold references to the logic vector This makes the code clearer by separating configuration (policy) from execution (visitor). The policy's operator() creates a visitor with the necessary references, and build_logic coordinates the process. Benefits: - Policies are now truly immutable and can be reused - No more confusing move semantics or rvalue references - Clear separation between factory and worker - Maintains the same CRTP pattern for postfix/infix variants Assisted-by: GitHub Copilot (Claude Sonnet 4.5) --- src/orange/orangeinp/ScaleUtils.cc | 8 +- src/orange/orangeinp/UnitProto.cc | 5 +- src/orange/orangeinp/detail/CsgLogicUtils.hh | 209 ++++++++++++------- test/orange/orangeinp/CsgTreeUtils.test.cc | 36 ++-- 4 files changed, 164 insertions(+), 94 deletions(-) diff --git a/src/orange/orangeinp/ScaleUtils.cc b/src/orange/orangeinp/ScaleUtils.cc index 81f63d2298..a735cd38a3 100644 --- a/src/orange/orangeinp/ScaleUtils.cc +++ b/src/orange/orangeinp/ScaleUtils.cc @@ -20,9 +20,11 @@ std::vector build_postfix_logic(CsgTree const& tree, NodeId n) { using celeritas::orangeinp::detail::PostfixBuildLogicPolicy; - PostfixBuildLogicPolicy policy{tree}; - policy(n); - return std::move(policy).logic(); + PostfixBuildLogicPolicy const policy{tree}; + std::vector logic; + auto visitor = policy(logic); + visitor(n); + return logic; } //---------------------------------------------------------------------------// diff --git a/src/orange/orangeinp/UnitProto.cc b/src/orange/orangeinp/UnitProto.cc index ad916c5cc0..bfb8e3297e 100644 --- a/src/orange/orangeinp/UnitProto.cc +++ b/src/orange/orangeinp/UnitProto.cc @@ -329,11 +329,12 @@ void UnitProto::build(ProtoBuilder& pb) const NodeId node_id = unit_volumes[vol_idx]; VolumeInput vi; // Construct logic and faces with remapped surfaces + detail::PostfixBuildLogicPolicy const policy{csg_unit.tree, + sorted_local_surfaces}; auto&& [faces, logic] = detail::build_logic( // always use postfix logic for unit input, post-processing to // convert to tracking notation - detail::PostfixBuildLogicPolicy{csg_unit.tree, - sorted_local_surfaces}, + policy, node_id); vi.faces = std::move(faces); vi.logic = std::move(logic); diff --git a/src/orange/orangeinp/detail/CsgLogicUtils.hh b/src/orange/orangeinp/detail/CsgLogicUtils.hh index 87dc298c6e..c982bfb9d9 100644 --- a/src/orange/orangeinp/detail/CsgLogicUtils.hh +++ b/src/orange/orangeinp/detail/CsgLogicUtils.hh @@ -80,36 +80,33 @@ inline BuildLogicResult::VecSurface remap_faces(BuildLogicResult::VecLogic& lgc) * the surfaces remapped to the index of the surface in the face vector. * * The function is templated on a policy class that determines the logic - * representation. The policy class must have an operator() that takes a - * NodeId. + * representation. The policy acts as a factory that creates a visitor to build + * the logic expression. * * The per-node local surfaces (faces) are sorted in ascending order of ID, not * of access, since they're always evaluated sequentially rather than as part * of the logic evaluation itself. */ template -inline BuildLogicResult build_logic(BuildLogicPolicy&& policy, NodeId n) +inline BuildLogicResult build_logic(BuildLogicPolicy const& policy, NodeId n) { - static_assert(std::is_invocable_v); - static_assert(std::is_rvalue_reference_v, - "Will move from policy: rvalue ref expected"); - CELER_EXPECT(policy.empty()); - // Construct logic vector as local surface IDs - policy(n); - auto lgc = std::forward(policy).logic(); + BuildLogicResult::VecLogic lgc; + auto visitor = policy(lgc); + visitor(n); return {remap_faces(lgc), std::move(lgc)}; } //---------------------------------------------------------------------------// /*! - * Base class for logic builder policies following CRTP pattern. + * Base class for logic builder visitors following CRTP pattern. * + * Visitors recursively traverse the CSG tree and append to a logic vector. * The call operator for Negated and Joined are not implemented in the base - * policy and must be provided by the derived class. + * visitor and must be provided by the derived class. */ -template -class BaseBuildLogicPolicy +template +class BaseBuildLogicVisitor { public: //!@{ @@ -124,9 +121,11 @@ class BaseBuildLogicPolicy public: // Construct without mapping - explicit inline BaseBuildLogicPolicy(CsgTree const& tree); + inline BaseBuildLogicVisitor(CsgTree const& tree, VecLogic& logic); // Construct with optional mapping - inline BaseBuildLogicPolicy(CsgTree const& tree, VecSurface const& vs); + inline BaseBuildLogicVisitor(CsgTree const& tree, + VecLogic& logic, + VecSurface const& vs); //! Build from a node ID inline void operator()(NodeId const& n); @@ -143,31 +142,26 @@ class BaseBuildLogicPolicy inline void operator()(Aliased const&); //!@} - //! Whether no logic has been filled - bool empty() const { return logic_.empty(); } - - //! Access the logic expression - VecLogic&& logic() && { return std::move(logic_); } - protected: //! Access the logic expression directly - VecLogic& logic() & { return logic_; } + VecLogic& logic() { return logic_; } private: ContainerVisitor visit_node_; VecSurface const* mapping_{nullptr}; - VecLogic logic_; + VecLogic& logic_; }; //---------------------------------------------------------------------------// /*! * Construct without mapping. */ -template -BaseBuildLogicPolicy::BaseBuildLogicPolicy(CsgTree const& tree) - : visit_node_{tree} +template +BaseBuildLogicVisitor::BaseBuildLogicVisitor(CsgTree const& tree, + VecLogic& logic) + : visit_node_{tree}, logic_{logic} { - static_assert(std::is_base_of_v, + static_assert(std::is_base_of_v, "CRTP: template parameter must be derived class"); } @@ -179,12 +173,12 @@ BaseBuildLogicPolicy::BaseBuildLogicPolicy(CsgTree const& tree) * Those surface IDs will be replaced by the index in the array. All existing * surface IDs must be present! */ -template -BaseBuildLogicPolicy::BaseBuildLogicPolicy(CsgTree const& tree, - VecSurface const& vs) - : visit_node_{tree}, mapping_{&vs} +template +BaseBuildLogicVisitor::BaseBuildLogicVisitor( + CsgTree const& tree, VecLogic& logic, VecSurface const& vs) + : visit_node_{tree}, mapping_{&vs}, logic_{logic} { - static_assert(std::is_base_of_v, + static_assert(std::is_base_of_v, "CRTP: template parameter must be derived class"); } @@ -192,18 +186,18 @@ BaseBuildLogicPolicy::BaseBuildLogicPolicy(CsgTree const& tree, /*! * Build from a node ID. */ -template -void BaseBuildLogicPolicy::operator()(NodeId const& n) +template +void BaseBuildLogicVisitor::operator()(NodeId const& n) { - visit_node_(static_cast(*this), n); + visit_node_(static_cast(*this), n); } //---------------------------------------------------------------------------// /*! * Append the "true" token. */ -template -void BaseBuildLogicPolicy::operator()(True const&) +template +void BaseBuildLogicVisitor::operator()(True const&) { logic_.push_back(logic::ltrue); } @@ -214,8 +208,8 @@ void BaseBuildLogicPolicy::operator()(True const&) * * The 'false' standin is always aliased to "not true" in the CSG tree. */ -template -void BaseBuildLogicPolicy::operator()(False const&) +template +void BaseBuildLogicVisitor::operator()(False const&) { CELER_ASSERT_UNREACHABLE(); } @@ -224,8 +218,8 @@ void BaseBuildLogicPolicy::operator()(False const&) /*! * Push a surface ID. */ -template -void BaseBuildLogicPolicy::operator()(Surface const& s) +template +void BaseBuildLogicVisitor::operator()(Surface const& s) { CELER_EXPECT(s.id < logic::lbegin); // Get index of original surface or remapped @@ -253,41 +247,30 @@ void BaseBuildLogicPolicy::operator()(Surface const& s) * Aliased node shouldn't be reachable if the tree is fully simplified, but * could be reachable for testing purposes. */ -template -void BaseBuildLogicPolicy::operator()(Aliased const& n) +template +void BaseBuildLogicVisitor::operator()(Aliased const& n) { (*this)(n.node); } //---------------------------------------------------------------------------// /*! - * Recursively construct a logic vector from a node with postfix operation. - * - * This is a policy used as template parameter of the \c build_logic function. - * The user invokes this class with a node ID (usually representing a cell), - * and then this class recurses into the daughters using a tree visitor. + * Visitor for constructing logic in postfix notation. * * Example: \verbatim - all(1, 3, 5) -> {{1, 3, 5}, "0 1 & 2 & &"} - all(1, 3, !all(2, 4)) -> {{1, 2, 3, 4}, "0 2 & 1 3 & ~ &"} + all(1, 3, 5) -> "0 1 & 2 & &" + all(1, 3, !all(2, 4)) -> "0 2 & 1 3 & ~ &" * \endverbatim */ -class PostfixBuildLogicPolicy - : public BaseBuildLogicPolicy +class PostfixBuildLogicVisitor + : public BaseBuildLogicVisitor { public: - //!@{ - //! \name Type aliases - using BaseBuildLogicPolicy::VecLogic; - using BaseBuildLogicPolicy::VecSurface; - //!@} - - public: - using BaseBuildLogicPolicy::BaseBuildLogicPolicy; + using BaseBuildLogicVisitor::BaseBuildLogicVisitor; //!@{ //! \name Visit a node directly - using BaseBuildLogicPolicy::operator(); + using BaseBuildLogicVisitor::operator(); //! Visit a negated node and append 'not'. void operator()(Negated const& n) @@ -316,32 +299,67 @@ class PostfixBuildLogicPolicy //---------------------------------------------------------------------------// /*! - * Recursively construct a logic vector from a node with infix operation. + * Policy for building logic in postfix notation. * - * This is a policy used as template parameter of \c build_logic. - * The user invokes this class with a node ID (usually representing a cell), - * and then this class recurses into the daughters using a tree visitor. + * This immutable factory creates visitors that construct logic expressions + * in postfix notation. It can be passed by const reference to \c build_logic. * * Example: \verbatim - all(1, 3, 5) -> {{1, 3, 5}, "(0 & 1 & 2)"} - all(1, 3, any(~(2), ~(4))) -> {{1, 2, 3, 4}, "(0 & 2 & (~1 | ~3))"} + all(1, 3, 5) -> {{1, 3, 5}, "0 1 & 2 & &"} + all(1, 3, !all(2, 4)) -> {{1, 2, 3, 4}, "0 2 & 1 3 & ~ &"} * \endverbatim */ -class InfixBuildLogicPolicy : public BaseBuildLogicPolicy +class PostfixBuildLogicPolicy { public: //!@{ //! \name Type aliases - using BaseBuildLogicPolicy::VecLogic; - using BaseBuildLogicPolicy::VecSurface; + using VecLogic = std::vector; + using VecSurface = std::vector; //!@} public: - using BaseBuildLogicPolicy::BaseBuildLogicPolicy; + // Construct without mapping + explicit PostfixBuildLogicPolicy(CsgTree const& tree) : tree_{tree} {} + // Construct with optional mapping + PostfixBuildLogicPolicy(CsgTree const& tree, VecSurface const& vs) + : tree_{tree}, mapping_{&vs} + { + } + + //! Create a visitor for building logic + auto operator()(VecLogic& logic) const + { + if (mapping_) + { + return PostfixBuildLogicVisitor{tree_, logic, *mapping_}; + } + return PostfixBuildLogicVisitor{tree_, logic}; + } + + private: + CsgTree const& tree_; + VecSurface const* mapping_{nullptr}; +}; + +//---------------------------------------------------------------------------// +/*! + * Visitor for constructing logic in infix notation. + * + * Example: \verbatim + all(1, 3, 5) -> "(0 & 1 & 2)" + all(1, 3, any(~(2), ~(4))) -> "(0 & 2 & (~1 | ~3))" + * \endverbatim + */ +class InfixBuildLogicVisitor + : public BaseBuildLogicVisitor +{ + public: + using BaseBuildLogicVisitor::BaseBuildLogicVisitor; //!@{ //! \name Visit a node directly - using BaseBuildLogicPolicy::operator(); + using BaseBuildLogicVisitor::operator(); //! Append 'not' and visit a negated node. void operator()(Negated const& n) @@ -370,6 +388,51 @@ class InfixBuildLogicPolicy : public BaseBuildLogicPolicy //!@} }; +//---------------------------------------------------------------------------// +/*! + * Policy for building logic in infix notation. + * + * This immutable factory creates visitors that construct logic expressions + * in infix notation. It can be passed by const reference to \c build_logic. + * + * Example: \verbatim + all(1, 3, 5) -> {{1, 3, 5}, "(0 & 1 & 2)"} + all(1, 3, any(~(2), ~(4))) -> {{1, 2, 3, 4}, "(0 & 2 & (~1 | ~3))"} + * \endverbatim + */ +class InfixBuildLogicPolicy +{ + public: + //!@{ + //! \name Type aliases + using VecLogic = std::vector; + using VecSurface = std::vector; + //!@} + + public: + // Construct without mapping + explicit InfixBuildLogicPolicy(CsgTree const& tree) : tree_{tree} {} + // Construct with optional mapping + InfixBuildLogicPolicy(CsgTree const& tree, VecSurface const& vs) + : tree_{tree}, mapping_{&vs} + { + } + + //! Create a visitor for building logic + auto operator()(VecLogic& logic) const + { + if (mapping_) + { + return InfixBuildLogicVisitor{tree_, logic, *mapping_}; + } + return InfixBuildLogicVisitor{tree_, logic}; + } + + private: + CsgTree const& tree_; + VecSurface const* mapping_{nullptr}; +}; + //---------------------------------------------------------------------------// } // namespace detail } // namespace orangeinp diff --git a/test/orange/orangeinp/CsgTreeUtils.test.cc b/test/orange/orangeinp/CsgTreeUtils.test.cc index 4ec87d5f82..41c5205b42 100644 --- a/test/orange/orangeinp/CsgTreeUtils.test.cc +++ b/test/orange/orangeinp/CsgTreeUtils.test.cc @@ -132,14 +132,16 @@ TEST_F(CsgTreeUtilsTest, postfix_simplify) // Test postfix and internal surface flagger InternalSurfaceFlagger has_internal_surfaces(tree_); - auto build_postfix = - [&](N n, detail::BuildLogicResult::VecSurface* mapping = nullptr) { - if (mapping) - { - return build_logic(PostfixBuildLogicPolicy{tree_, *mapping}, n); - } - return build_logic(PostfixBuildLogicPolicy{tree_}, n); - }; + auto build_postfix + = [&](N n, detail::BuildLogicResult::VecSurface* mapping = nullptr) { + if (mapping) + { + PostfixBuildLogicPolicy const policy{tree_, *mapping}; + return build_logic(policy, n); + } + PostfixBuildLogicPolicy const policy{tree_}; + return build_logic(policy, n); + }; { EXPECT_FALSE(has_internal_surfaces(mz)); @@ -305,14 +307,16 @@ TEST_F(CsgTreeUtilsTest, infix_simplify) // Test infix and internal surface flagger InternalSurfaceFlagger has_internal_surfaces(tree_); - auto build_infix = - [&](N n, detail::BuildLogicResult::VecSurface* mapping = nullptr) { - if (mapping) - { - return build_logic(InfixBuildLogicPolicy{tree_, *mapping}, n); - } - return build_logic(InfixBuildLogicPolicy{tree_}, n); - }; + auto build_infix + = [&](N n, detail::BuildLogicResult::VecSurface* mapping = nullptr) { + if (mapping) + { + InfixBuildLogicPolicy const policy{tree_, *mapping}; + return build_logic(policy, n); + } + InfixBuildLogicPolicy const policy{tree_}; + return build_logic(policy, n); + }; { EXPECT_FALSE(has_internal_surfaces(mz)); auto&& [faces, lgc] = build_infix(mz); From 15798f79b7110dd1c4047d3216a7331be8cc720d Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Thu, 5 Feb 2026 18:53:49 -0500 Subject: [PATCH 08/24] Move policy out of loop --- src/orange/orangeinp/UnitProto.cc | 14 ++- src/orange/orangeinp/detail/CsgLogicUtils.hh | 89 +++++++++----------- 2 files changed, 46 insertions(+), 57 deletions(-) diff --git a/src/orange/orangeinp/UnitProto.cc b/src/orange/orangeinp/UnitProto.cc index bfb8e3297e..ca4f6cf59e 100644 --- a/src/orange/orangeinp/UnitProto.cc +++ b/src/orange/orangeinp/UnitProto.cc @@ -324,18 +324,16 @@ void UnitProto::build(ProtoBuilder& pb) const result.volumes.reserve(unit_volumes.size() + static_cast(csg_unit.background)); + // Always use postfix logic for unit input, post-processing to + // convert to tracking notation + detail::PostfixBuildLogicPolicy const policy{csg_unit.tree, + sorted_local_surfaces}; + // Construct logic and faces with remapped surfaces for (auto vol_idx : range(unit_volumes.size())) { NodeId node_id = unit_volumes[vol_idx]; VolumeInput vi; - // Construct logic and faces with remapped surfaces - detail::PostfixBuildLogicPolicy const policy{csg_unit.tree, - sorted_local_surfaces}; - auto&& [faces, logic] = detail::build_logic( - // always use postfix logic for unit input, post-processing to - // convert to tracking notation - policy, - node_id); + auto&& [faces, logic] = detail::build_logic(policy, node_id); vi.faces = std::move(faces); vi.logic = std::move(logic); // Set bounding box diff --git a/src/orange/orangeinp/detail/CsgLogicUtils.hh b/src/orange/orangeinp/detail/CsgLogicUtils.hh index c982bfb9d9..61b72e19af 100644 --- a/src/orange/orangeinp/detail/CsgLogicUtils.hh +++ b/src/orange/orangeinp/detail/CsgLogicUtils.hh @@ -105,8 +105,8 @@ inline BuildLogicResult build_logic(BuildLogicPolicy const& policy, NodeId n) * The call operator for Negated and Joined are not implemented in the base * visitor and must be provided by the derived class. */ -template -class BaseBuildLogicVisitor +template +class BaseLogicBuilder { public: //!@{ @@ -121,12 +121,11 @@ class BaseBuildLogicVisitor public: // Construct without mapping - inline BaseBuildLogicVisitor(CsgTree const& tree, VecLogic& logic); + inline BaseLogicBuilder(CsgTree const& tree, VecLogic& logic); // Construct with optional mapping - inline BaseBuildLogicVisitor(CsgTree const& tree, - VecLogic& logic, - VecSurface const& vs); - + inline BaseLogicBuilder(CsgTree const& tree, + VecLogic& logic, + VecSurface const& vs); //! Build from a node ID inline void operator()(NodeId const& n); @@ -143,26 +142,21 @@ class BaseBuildLogicVisitor //!@} protected: - //! Access the logic expression directly - VecLogic& logic() { return logic_; } + VecLogic& logic_; private: ContainerVisitor visit_node_; VecSurface const* mapping_{nullptr}; - VecLogic& logic_; }; //---------------------------------------------------------------------------// /*! * Construct without mapping. */ -template -BaseBuildLogicVisitor::BaseBuildLogicVisitor(CsgTree const& tree, - VecLogic& logic) - : visit_node_{tree}, logic_{logic} +template +BaseLogicBuilder::BaseLogicBuilder(CsgTree const& tree, VecLogic& logic) + : logic_{logic}, visit_node_{tree} { - static_assert(std::is_base_of_v, - "CRTP: template parameter must be derived class"); } //---------------------------------------------------------------------------// @@ -173,31 +167,30 @@ BaseBuildLogicVisitor::BaseBuildLogicVisitor(CsgTree const& tree * Those surface IDs will be replaced by the index in the array. All existing * surface IDs must be present! */ -template -BaseBuildLogicVisitor::BaseBuildLogicVisitor( - CsgTree const& tree, VecLogic& logic, VecSurface const& vs) - : visit_node_{tree}, mapping_{&vs}, logic_{logic} +template +BaseLogicBuilder::BaseLogicBuilder(CsgTree const& tree, + VecLogic& logic, + VecSurface const& vs) + : logic_{logic}, visit_node_{tree}, mapping_{&vs} { - static_assert(std::is_base_of_v, - "CRTP: template parameter must be derived class"); } //---------------------------------------------------------------------------// /*! * Build from a node ID. */ -template -void BaseBuildLogicVisitor::operator()(NodeId const& n) +template +void BaseLogicBuilder::operator()(NodeId const& n) { - visit_node_(static_cast(*this), n); + visit_node_(static_cast(*this), n); } //---------------------------------------------------------------------------// /*! * Append the "true" token. */ -template -void BaseBuildLogicVisitor::operator()(True const&) +template +void BaseLogicBuilder::operator()(True const&) { logic_.push_back(logic::ltrue); } @@ -208,8 +201,8 @@ void BaseBuildLogicVisitor::operator()(True const&) * * The 'false' standin is always aliased to "not true" in the CSG tree. */ -template -void BaseBuildLogicVisitor::operator()(False const&) +template +void BaseLogicBuilder::operator()(False const&) { CELER_ASSERT_UNREACHABLE(); } @@ -218,8 +211,8 @@ void BaseBuildLogicVisitor::operator()(False const&) /*! * Push a surface ID. */ -template -void BaseBuildLogicVisitor::operator()(Surface const& s) +template +void BaseLogicBuilder::operator()(Surface const& s) { CELER_EXPECT(s.id < logic::lbegin); // Get index of original surface or remapped @@ -247,8 +240,8 @@ void BaseBuildLogicVisitor::operator()(Surface const& s) * Aliased node shouldn't be reachable if the tree is fully simplified, but * could be reachable for testing purposes. */ -template -void BaseBuildLogicVisitor::operator()(Aliased const& n) +template +void BaseLogicBuilder::operator()(Aliased const& n) { (*this)(n.node); } @@ -262,21 +255,20 @@ void BaseBuildLogicVisitor::operator()(Aliased const& n) all(1, 3, !all(2, 4)) -> "0 2 & 1 3 & ~ &" * \endverbatim */ -class PostfixBuildLogicVisitor - : public BaseBuildLogicVisitor +class PostfixLogicBuilder : public BaseLogicBuilder { public: - using BaseBuildLogicVisitor::BaseBuildLogicVisitor; + using BaseLogicBuilder::BaseLogicBuilder; //!@{ //! \name Visit a node directly - using BaseBuildLogicVisitor::operator(); + using BaseLogicBuilder::operator(); //! Visit a negated node and append 'not'. void operator()(Negated const& n) { (*this)(n.node); - logic().push_back(logic::lnot); + logic_.push_back(logic::lnot); } //! Visit daughter nodes and append the conjunction. @@ -291,7 +283,7 @@ class PostfixBuildLogicVisitor while (iter != n.nodes.end()) { (*this)(*iter++); - logic().push_back(n.op); + logic_.push_back(n.op); } } //!@} @@ -332,9 +324,9 @@ class PostfixBuildLogicPolicy { if (mapping_) { - return PostfixBuildLogicVisitor{tree_, logic, *mapping_}; + return PostfixLogicBuilder{tree_, logic, *mapping_}; } - return PostfixBuildLogicVisitor{tree_, logic}; + return PostfixLogicBuilder{tree_, logic}; } private: @@ -351,20 +343,19 @@ class PostfixBuildLogicPolicy all(1, 3, any(~(2), ~(4))) -> "(0 & 2 & (~1 | ~3))" * \endverbatim */ -class InfixBuildLogicVisitor - : public BaseBuildLogicVisitor +class InfixLogicBuilder : public BaseLogicBuilder { public: - using BaseBuildLogicVisitor::BaseBuildLogicVisitor; + using BaseLogicBuilder::BaseLogicBuilder; //!@{ //! \name Visit a node directly - using BaseBuildLogicVisitor::operator(); + using BaseLogicBuilder::operator(); //! Append 'not' and visit a negated node. void operator()(Negated const& n) { - this->logic().push_back(logic::lnot); + logic_.push_back(logic::lnot); (*this)(n.node); } @@ -372,7 +363,7 @@ class InfixBuildLogicVisitor void operator()(Joined const& n) { CELER_EXPECT(n.nodes.size() > 1); - auto& logic = this->logic(); + auto& logic = logic_; logic.push_back(logic::lopen); // Visit first node, then add conjunction for subsequent nodes auto iter = n.nodes.begin(); @@ -423,9 +414,9 @@ class InfixBuildLogicPolicy { if (mapping_) { - return InfixBuildLogicVisitor{tree_, logic, *mapping_}; + return InfixLogicBuilder{tree_, logic, *mapping_}; } - return InfixBuildLogicVisitor{tree_, logic}; + return InfixLogicBuilder{tree_, logic}; } private: From 5a5e1ef596df278b2e49c6bd91e5c3c7aaaae952 Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Thu, 5 Feb 2026 19:05:12 -0500 Subject: [PATCH 09/24] Simplify --- src/orange/orangeinp/detail/CsgLogicUtils.hh | 103 +++++-------------- 1 file changed, 24 insertions(+), 79 deletions(-) diff --git a/src/orange/orangeinp/detail/CsgLogicUtils.hh b/src/orange/orangeinp/detail/CsgLogicUtils.hh index 61b72e19af..2b9a35a9a2 100644 --- a/src/orange/orangeinp/detail/CsgLogicUtils.hh +++ b/src/orange/orangeinp/detail/CsgLogicUtils.hh @@ -87,8 +87,8 @@ inline BuildLogicResult::VecSurface remap_faces(BuildLogicResult::VecLogic& lgc) * of access, since they're always evaluated sequentially rather than as part * of the logic evaluation itself. */ -template -inline BuildLogicResult build_logic(BuildLogicPolicy const& policy, NodeId n) +template +inline BuildLogicResult build_logic(LogicPolicy const& policy, NodeId n) { // Construct logic vector as local surface IDs BuildLogicResult::VecLogic lgc; @@ -120,12 +120,10 @@ class BaseLogicBuilder "face and surface ints"); public: - // Construct without mapping - inline BaseLogicBuilder(CsgTree const& tree, VecLogic& logic); - // Construct with optional mapping + // Construct with optional mapping pointer inline BaseLogicBuilder(CsgTree const& tree, VecLogic& logic, - VecSurface const& vs); + VecSurface const* vs = nullptr); //! Build from a node ID inline void operator()(NodeId const& n); @@ -151,17 +149,7 @@ class BaseLogicBuilder //---------------------------------------------------------------------------// /*! - * Construct without mapping. - */ -template -BaseLogicBuilder::BaseLogicBuilder(CsgTree const& tree, VecLogic& logic) - : logic_{logic}, visit_node_{tree} -{ -} - -//---------------------------------------------------------------------------// -/*! - * Construct with optional mapping. + * Construct with optional mapping pointer. * * The optional surface mapping is an ordered vector of *existing* surface IDs. * Those surface IDs will be replaced by the index in the array. All existing @@ -170,8 +158,8 @@ BaseLogicBuilder::BaseLogicBuilder(CsgTree const& tree, VecLogic& logic) template BaseLogicBuilder::BaseLogicBuilder(CsgTree const& tree, VecLogic& logic, - VecSurface const& vs) - : logic_{logic}, visit_node_{tree}, mapping_{&vs} + VecSurface const* vs) + : logic_{logic}, visit_node_{tree}, mapping_{vs} { } @@ -289,51 +277,6 @@ class PostfixLogicBuilder : public BaseLogicBuilder //!@} }; -//---------------------------------------------------------------------------// -/*! - * Policy for building logic in postfix notation. - * - * This immutable factory creates visitors that construct logic expressions - * in postfix notation. It can be passed by const reference to \c build_logic. - * - * Example: \verbatim - all(1, 3, 5) -> {{1, 3, 5}, "0 1 & 2 & &"} - all(1, 3, !all(2, 4)) -> {{1, 2, 3, 4}, "0 2 & 1 3 & ~ &"} - * \endverbatim - */ -class PostfixBuildLogicPolicy -{ - public: - //!@{ - //! \name Type aliases - using VecLogic = std::vector; - using VecSurface = std::vector; - //!@} - - public: - // Construct without mapping - explicit PostfixBuildLogicPolicy(CsgTree const& tree) : tree_{tree} {} - // Construct with optional mapping - PostfixBuildLogicPolicy(CsgTree const& tree, VecSurface const& vs) - : tree_{tree}, mapping_{&vs} - { - } - - //! Create a visitor for building logic - auto operator()(VecLogic& logic) const - { - if (mapping_) - { - return PostfixLogicBuilder{tree_, logic, *mapping_}; - } - return PostfixLogicBuilder{tree_, logic}; - } - - private: - CsgTree const& tree_; - VecSurface const* mapping_{nullptr}; -}; - //---------------------------------------------------------------------------// /*! * Visitor for constructing logic in infix notation. @@ -381,17 +324,16 @@ class InfixLogicBuilder : public BaseLogicBuilder //---------------------------------------------------------------------------// /*! - * Policy for building logic in infix notation. + * Policy for building logic expressions. * - * This immutable factory creates visitors that construct logic expressions - * in infix notation. It can be passed by const reference to \c build_logic. + * This immutable factory creates visitors that construct logic expressions. + * It can be passed by const reference to \c build_logic. * - * Example: \verbatim - all(1, 3, 5) -> {{1, 3, 5}, "(0 & 1 & 2)"} - all(1, 3, any(~(2), ~(4))) -> {{1, 2, 3, 4}, "(0 & 2 & (~1 | ~3))"} - * \endverbatim + * \tparam LogicBuilder The builder type (PostfixLogicBuilder or + * InfixLogicBuilder) */ -class InfixBuildLogicPolicy +template +class BuildLogicPolicy { public: //!@{ @@ -402,9 +344,9 @@ class InfixBuildLogicPolicy public: // Construct without mapping - explicit InfixBuildLogicPolicy(CsgTree const& tree) : tree_{tree} {} + explicit BuildLogicPolicy(CsgTree const& tree) : tree_{tree} {} // Construct with optional mapping - InfixBuildLogicPolicy(CsgTree const& tree, VecSurface const& vs) + BuildLogicPolicy(CsgTree const& tree, VecSurface const& vs) : tree_{tree}, mapping_{&vs} { } @@ -412,11 +354,7 @@ class InfixBuildLogicPolicy //! Create a visitor for building logic auto operator()(VecLogic& logic) const { - if (mapping_) - { - return InfixLogicBuilder{tree_, logic, *mapping_}; - } - return InfixLogicBuilder{tree_, logic}; + return LogicBuilder{tree_, logic, mapping_}; } private: @@ -424,6 +362,13 @@ class InfixBuildLogicPolicy VecSurface const* mapping_{nullptr}; }; +//---------------------------------------------------------------------------// +/*! + * Policy classes are factories. + */ +using PostfixBuildLogicPolicy = BuildLogicPolicy; +using InfixBuildLogicPolicy = BuildLogicPolicy; + //---------------------------------------------------------------------------// } // namespace detail } // namespace orangeinp From 7165e2184d08c7b74705c9367b626f310d44d344 Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Thu, 5 Feb 2026 19:17:04 -0500 Subject: [PATCH 10/24] Add runtime policy --- src/orange/orangeinp/UnitProto.cc | 4 +- src/orange/orangeinp/detail/CsgLogicUtils.hh | 119 ++++++++++++++----- 2 files changed, 94 insertions(+), 29 deletions(-) diff --git a/src/orange/orangeinp/UnitProto.cc b/src/orange/orangeinp/UnitProto.cc index ca4f6cf59e..6570645984 100644 --- a/src/orange/orangeinp/UnitProto.cc +++ b/src/orange/orangeinp/UnitProto.cc @@ -326,8 +326,8 @@ void UnitProto::build(ProtoBuilder& pb) const // Always use postfix logic for unit input, post-processing to // convert to tracking notation - detail::PostfixBuildLogicPolicy const policy{csg_unit.tree, - sorted_local_surfaces}; + detail::RuntimeBuildLogicPolicy const policy{ + LogicNotation::postfix, csg_unit.tree, sorted_local_surfaces}; // Construct logic and faces with remapped surfaces for (auto vol_idx : range(unit_volumes.size())) { diff --git a/src/orange/orangeinp/detail/CsgLogicUtils.hh b/src/orange/orangeinp/detail/CsgLogicUtils.hh index 2b9a35a9a2..f1ea081709 100644 --- a/src/orange/orangeinp/detail/CsgLogicUtils.hh +++ b/src/orange/orangeinp/detail/CsgLogicUtils.hh @@ -8,6 +8,7 @@ #include #include +#include #include #include "corecel/Assert.hh" @@ -71,32 +72,6 @@ inline BuildLogicResult::VecSurface remap_faces(BuildLogicResult::VecLogic& lgc) return faces; } -//---------------------------------------------------------------------------// -/*! - * Construct a logic representation of a node. - * - * The result is a pair of vectors: the sorted surface IDs comprising the faces - * of this volume, and the logical representation using \em face IDs, i.e. with - * the surfaces remapped to the index of the surface in the face vector. - * - * The function is templated on a policy class that determines the logic - * representation. The policy acts as a factory that creates a visitor to build - * the logic expression. - * - * The per-node local surfaces (faces) are sorted in ascending order of ID, not - * of access, since they're always evaluated sequentially rather than as part - * of the logic evaluation itself. - */ -template -inline BuildLogicResult build_logic(LogicPolicy const& policy, NodeId n) -{ - // Construct logic vector as local surface IDs - BuildLogicResult::VecLogic lgc; - auto visitor = policy(lgc); - visitor(n); - return {remap_faces(lgc), std::move(lgc)}; -} - //---------------------------------------------------------------------------// /*! * Base class for logic builder visitors following CRTP pattern. @@ -322,6 +297,44 @@ class InfixLogicBuilder : public BaseLogicBuilder //!@} }; +//---------------------------------------------------------------------------// +/*! + * Construct a logic representation of a node. + * + * The result is a pair of vectors: the sorted surface IDs comprising the faces + * of this volume, and the logical representation using \em face IDs, i.e. with + * the surfaces remapped to the index of the surface in the face vector. + * + * The function is templated on a policy class that determines the logic + * representation. The policy acts as a factory that creates a visitor to build + * the logic expression. + * + * The per-node local surfaces (faces) are sorted in ascending order of ID, not + * of access, since they're always evaluated sequentially rather than as part + * of the logic evaluation itself. + */ +template +inline BuildLogicResult build_logic(LogicPolicy const& policy, NodeId n) +{ + // Construct logic vector as local surface IDs + BuildLogicResult::VecLogic lgc; + auto visitor = policy(lgc); + + // Handle both direct builders and variant-wrapped builders + if constexpr (std::is_same_v< + decltype(visitor), + std::variant>) + { + std::visit([n](auto& v) { v(n); }, visitor); + } + else + { + visitor(n); + } + + return {remap_faces(lgc), std::move(lgc)}; +} + //---------------------------------------------------------------------------// /*! * Policy for building logic expressions. @@ -345,7 +358,7 @@ class BuildLogicPolicy public: // Construct without mapping explicit BuildLogicPolicy(CsgTree const& tree) : tree_{tree} {} - // Construct with optional mapping + // Construct with mapping BuildLogicPolicy(CsgTree const& tree, VecSurface const& vs) : tree_{tree}, mapping_{&vs} { @@ -369,6 +382,58 @@ class BuildLogicPolicy using PostfixBuildLogicPolicy = BuildLogicPolicy; using InfixBuildLogicPolicy = BuildLogicPolicy; +//---------------------------------------------------------------------------// +/*! + * Runtime-dispatching policy for building logic expressions. + * + * This policy class selects between postfix and infix notation at runtime + * based on a LogicNotation enum value. The operator() returns a variant + * containing the appropriate builder type. + */ +class RuntimeBuildLogicPolicy +{ + public: + //!@{ + //! \name Type aliases + using VecLogic = std::vector; + using VecSurface = std::vector; + using Builder = std::variant; + //!@} + + public: + // Construct without mapping + RuntimeBuildLogicPolicy(LogicNotation notation, CsgTree const& tree) + : notation_{notation}, tree_{tree} + { + } + // Construct with mapping + RuntimeBuildLogicPolicy(LogicNotation notation, + CsgTree const& tree, + VecSurface const& vs) + : notation_{notation}, tree_{tree}, mapping_{&vs} + { + } + + //! Create a visitor for building logic + Builder operator()(VecLogic& logic) const + { + switch (notation_) + { + case LogicNotation::postfix: + return PostfixLogicBuilder{tree_, logic, mapping_}; + case LogicNotation::infix: + return InfixLogicBuilder{tree_, logic, mapping_}; + default: + CELER_ASSERT_UNREACHABLE(); + } + } + + private: + LogicNotation notation_; + CsgTree const& tree_; + VecSurface const* mapping_{nullptr}; +}; + //---------------------------------------------------------------------------// } // namespace detail } // namespace orangeinp From c00fd0c4e4ebcbd26bdbf4414d00874d7634b1ec Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Fri, 6 Feb 2026 04:36:57 -0500 Subject: [PATCH 11/24] Convert more token vectors to string --- test/orange/orangeinp/CsgTreeUtils.test.cc | 99 +++++----------------- 1 file changed, 20 insertions(+), 79 deletions(-) diff --git a/test/orange/orangeinp/CsgTreeUtils.test.cc b/test/orange/orangeinp/CsgTreeUtils.test.cc index 41c5205b42..19f9c292a2 100644 --- a/test/orange/orangeinp/CsgTreeUtils.test.cc +++ b/test/orange/orangeinp/CsgTreeUtils.test.cc @@ -7,6 +7,7 @@ #include "orange/orangeinp/CsgTreeUtils.hh" #include "orange/OrangeTypes.hh" +#include "orange/detail/LogicUtils.hh" #include "orange/orangeinp/CsgTree.hh" #include "orange/orangeinp/CsgTypes.hh" #include "orange/orangeinp/detail/CsgLogicUtils.hh" @@ -20,6 +21,7 @@ using N = celeritas::orangeinp::NodeId; using S = celeritas::LocalSurfaceId; +using celeritas::detail::string_to_logic; using celeritas::orangeinp::detail::build_logic; using celeritas::orangeinp::detail::InfixBuildLogicPolicy; using celeritas::orangeinp::detail::InternalSurfaceFlagger; @@ -147,7 +149,7 @@ TEST_F(CsgTreeUtilsTest, postfix_simplify) EXPECT_FALSE(has_internal_surfaces(mz)); auto&& [faces, lgc] = build_postfix(mz); - static size_type expected_lgc[] = {0}; + auto const expected_lgc = string_to_logic("0"); static S const expected_faces[] = {S{0u}}; EXPECT_VEC_EQ(expected_lgc, lgc); EXPECT_VEC_EQ(expected_faces, faces); @@ -162,7 +164,7 @@ TEST_F(CsgTreeUtilsTest, postfix_simplify) EXPECT_FALSE(has_internal_surfaces(below_pz)); auto&& [faces, lgc] = build_postfix(below_pz); - static size_type expected_lgc[] = {0, logic::lnot}; + auto const expected_lgc = string_to_logic("0~"); static S const expected_faces[] = {S{1u}}; EXPECT_VEC_EQ(expected_lgc, lgc); EXPECT_VEC_EQ(expected_faces, faces); @@ -175,8 +177,7 @@ TEST_F(CsgTreeUtilsTest, postfix_simplify) EXPECT_FALSE(has_internal_surfaces(zslab)); auto&& [faces, lgc] = build_postfix(zslab); - static size_type const expected_lgc[] - = {0u, 1u, logic::lnot, logic::land}; + auto const expected_lgc = string_to_logic("0 1~ &"); static S const expected_faces[] = {S{0u}, S{1u}}; EXPECT_VEC_EQ(expected_lgc, lgc); EXPECT_VEC_EQ(expected_faces, faces); @@ -189,8 +190,7 @@ TEST_F(CsgTreeUtilsTest, postfix_simplify) EXPECT_FALSE(has_internal_surfaces(inner_cyl)); auto&& [faces, lgc] = build_postfix(inner_cyl); - static size_type const expected_lgc[] - = {0u, 1u, logic::lnot, logic::land, 2u, logic::lnot, logic::land}; + auto const expected_lgc = string_to_logic("0 1~ & 2~ &"); static S const expected_faces[] = {S{0u}, S{1u}, S{2u}}; EXPECT_VEC_EQ(expected_lgc, lgc); EXPECT_VEC_EQ(expected_faces, faces); @@ -201,24 +201,8 @@ TEST_F(CsgTreeUtilsTest, postfix_simplify) EXPECT_TRUE(has_internal_surfaces(shell)); auto&& [faces, lgc] = build_postfix(shell); - static size_type const expected_lgc[] = { - 0u, - 1u, - logic::lnot, - logic::land, - 3u, - logic::lnot, - logic::land, - 0u, - 1u, - logic::lnot, - logic::land, - 2u, - logic::lnot, - logic::land, - logic::lnot, - logic::land, - }; + auto const expected_lgc + = string_to_logic("0 1~ & 3~ & 0 1~ & 2~ &~ &"); static S const expected_faces[] = {S{0u}, S{1u}, S{2u}, S{3u}}; EXPECT_VEC_EQ(expected_lgc, lgc); EXPECT_VEC_EQ(expected_faces, faces); @@ -236,8 +220,7 @@ TEST_F(CsgTreeUtilsTest, postfix_simplify) EXPECT_FALSE(has_internal_surfaces(bdy)); auto&& [faces, lgc] = build_postfix(bdy); - static size_type const expected_lgc[] - = {0u, 1u, logic::lnot, logic::land, 2u, logic::land}; + auto const expected_lgc = string_to_logic("0 1~ & 2&"); static S const expected_faces[] = {S{0u}, S{1u}, S{4u}}; EXPECT_VEC_EQ(expected_lgc, lgc); EXPECT_VEC_EQ(expected_faces, faces); @@ -247,12 +230,8 @@ TEST_F(CsgTreeUtilsTest, postfix_simplify) EXPECT_TRUE(has_internal_surfaces(always_false)); auto&& [faces, lgc] = build_postfix(always_false); - static size_type const expected_lgc[] = { - 0u, 1u, logic::lnot, logic::land, 2u, - logic::lnot, logic::land, 3u, logic::lnot, logic::land, - 0u, 1u, logic::lnot, logic::land, 2u, - logic::lnot, logic::land, logic::lnot, logic::land, - }; + auto const expected_lgc + = string_to_logic("0 1~ & 2~ & 3~ & 0 1~ & 2~ &~ &"); static S const expected_faces[] = {S{0}, S{1}, S{2}, S{3}}; EXPECT_VEC_EQ(expected_lgc, lgc) << ReprLogic{lgc}; EXPECT_VEC_EQ(expected_faces, faces); @@ -275,8 +254,7 @@ TEST_F(CsgTreeUtilsTest, postfix_simplify) auto&& [faces, lgc] = build_postfix(shell, &remapped_surf); - static size_type const expected_lgc[] - = {0u, 1u, logic::lnot, logic::land}; + auto const expected_lgc = string_to_logic("0 1~ &"); static S const expected_faces[] = {S{0u}, S{1u}}; EXPECT_VEC_EQ(expected_lgc, lgc); EXPECT_VEC_EQ(expected_faces, faces); @@ -321,7 +299,7 @@ TEST_F(CsgTreeUtilsTest, infix_simplify) EXPECT_FALSE(has_internal_surfaces(mz)); auto&& [faces, lgc] = build_infix(mz); - static size_type expected_lgc[] = {0}; + auto const expected_lgc = string_to_logic("0"); static S const expected_faces[] = {S{0u}}; EXPECT_VEC_EQ(expected_lgc, lgc); EXPECT_VEC_EQ(expected_faces, faces); @@ -336,7 +314,7 @@ TEST_F(CsgTreeUtilsTest, infix_simplify) EXPECT_FALSE(has_internal_surfaces(below_pz)); auto&& [faces, lgc] = build_infix(below_pz); - static size_type expected_lgc[] = {logic::lnot, 0}; + auto const expected_lgc = string_to_logic("~0"); static S const expected_faces[] = {S{1u}}; EXPECT_VEC_EQ(expected_lgc, lgc); EXPECT_VEC_EQ(expected_faces, faces); @@ -349,8 +327,7 @@ TEST_F(CsgTreeUtilsTest, infix_simplify) EXPECT_FALSE(has_internal_surfaces(zslab)); auto&& [faces, lgc] = build_infix(zslab); - static size_type const expected_lgc[] - = {logic::lopen, 0u, logic::land, logic::lnot, 1u, logic::lclose}; + auto const expected_lgc = string_to_logic("(0 & ~1)"); static S const expected_faces[] = {S{0u}, S{1u}}; EXPECT_VEC_EQ(expected_lgc, lgc); EXPECT_VEC_EQ(expected_faces, faces); @@ -363,15 +340,7 @@ TEST_F(CsgTreeUtilsTest, infix_simplify) EXPECT_FALSE(has_internal_surfaces(inner_cyl)); auto&& [faces, lgc] = build_infix(inner_cyl); - static size_type const expected_lgc[] = {logic::lopen, - 0u, - logic::land, - logic::lnot, - 1u, - logic::land, - logic::lnot, - 2u, - logic::lclose}; + auto const expected_lgc = string_to_logic("(0 & ~1 & ~2)"); static S const expected_faces[] = {S{0u}, S{1u}, S{2u}}; EXPECT_VEC_EQ(expected_lgc, lgc); EXPECT_VEC_EQ(expected_faces, faces); @@ -382,28 +351,8 @@ TEST_F(CsgTreeUtilsTest, infix_simplify) EXPECT_TRUE(has_internal_surfaces(shell)); auto&& [faces, lgc] = build_infix(shell); - static size_type const expected_lgc[] = { - logic::lopen, - 0u, - logic::land, - logic::lnot, - 1u, - logic::land, - logic::lnot, - 3u, - logic::land, - logic::lnot, - logic::lopen, - 0u, - logic::land, - logic::lnot, - 1u, - logic::land, - logic::lnot, - 2u, - logic::lclose, - logic::lclose, - }; + auto const expected_lgc + = string_to_logic("(0 & ~1 & ~3 & ~(0 & ~1 & ~2))"); static S const expected_faces[] = {S{0u}, S{1u}, S{2u}, S{3u}}; EXPECT_VEC_EQ(expected_lgc, lgc) << ReprLogic{lgc}; EXPECT_VEC_EQ(expected_faces, faces); @@ -421,14 +370,7 @@ TEST_F(CsgTreeUtilsTest, infix_simplify) EXPECT_FALSE(has_internal_surfaces(bdy)); auto&& [faces, lgc] = build_infix(bdy); - static size_type const expected_lgc[] = {logic::lopen, - 0u, - logic::land, - logic::lnot, - 1u, - logic::land, - 2u, - logic::lclose}; + auto const expected_lgc = string_to_logic("(0 & ~1 & 2)"); static S const expected_faces[] = {S{0u}, S{1u}, S{4u}}; EXPECT_VEC_EQ(expected_lgc, lgc); EXPECT_VEC_EQ(expected_faces, faces); @@ -449,8 +391,7 @@ TEST_F(CsgTreeUtilsTest, infix_simplify) EXPECT_VEC_EQ(expected_remapped_surf, remapped_surf); auto&& [faces, lgc] = build_infix(shell, &remapped_surf); - static size_type const expected_lgc[] - = {logic::lopen, 0u, logic::land, logic::lnot, 1u, logic::lclose}; + auto const expected_lgc = string_to_logic("(0 & ~1)"); static S const expected_faces[] = {S{0u}, S{1u}}; EXPECT_VEC_EQ(expected_lgc, lgc); EXPECT_VEC_EQ(expected_faces, faces); From afa1b1500bb359512f22173d982a4d31bc96cfe5 Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Fri, 6 Feb 2026 05:05:11 -0500 Subject: [PATCH 12/24] Remove ScaleUtils and improve conversion explanation --- src/orange/CMakeLists.txt | 1 - src/orange/detail/LogicUtils.cc | 24 ++++++++++++++++------ src/orange/orangeinp/ScaleUtils.cc | 32 ------------------------------ src/orange/orangeinp/ScaleUtils.hh | 24 ++-------------------- 4 files changed, 20 insertions(+), 61 deletions(-) delete mode 100644 src/orange/orangeinp/ScaleUtils.cc diff --git a/src/orange/CMakeLists.txt b/src/orange/CMakeLists.txt index 7bd46d75b8..4465b41290 100644 --- a/src/orange/CMakeLists.txt +++ b/src/orange/CMakeLists.txt @@ -50,7 +50,6 @@ list(APPEND SOURCES orangeinp/Shape.cc orangeinp/Solid.cc orangeinp/StackedExtrudedPolygon.cc - orangeinp/ScaleUtils.cc orangeinp/Transformed.cc orangeinp/Truncated.cc orangeinp/UnitProto.cc diff --git a/src/orange/detail/LogicUtils.cc b/src/orange/detail/LogicUtils.cc index eb2c6821d0..e03c11bdb9 100644 --- a/src/orange/detail/LogicUtils.cc +++ b/src/orange/detail/LogicUtils.cc @@ -11,7 +11,7 @@ #include "corecel/Assert.hh" #include "orange/orangeinp/CsgTree.hh" #include "orange/orangeinp/CsgTreeUtils.hh" -#include "orange/orangeinp/ScaleUtils.hh" +#include "orange/orangeinp/detail/CsgLogicUtils.hh" #include "../OrangeTypes.hh" @@ -88,11 +88,23 @@ orangeinp::CsgTree build_tree_from_postfix(Span postfix) std::vector simplify_negated_joins_postfix(Span postfix) { - auto tree = build_tree_from_postfix(postfix); - auto simplified = orangeinp::transform_negated_joins(tree); - CELER_ASSERT(!simplified.first.volumes().empty()); - auto root = simplified.first.volumes().front(); - return orangeinp::build_postfix_logic(simplified.first, root); + CELER_EXPECT(!postfix.empty()); + + using namespace orangeinp; + + // Construct a CSG tree from the input and simplify it + CsgTree tree + = transform_negated_joins(build_tree_from_postfix(postfix)).first; + CELER_ASSERT(tree.volumes().size() == 1); + NodeId root = tree.volumes().front(); + + // Convert simplified tree to postfix + orangeinp::detail::PostfixBuildLogicPolicy const policy{tree}; + std::vector logic; + auto build_impl = policy(logic); + build_impl(root); + CELER_ENSURE(!logic.empty()); + return logic; } //---------------------------------------------------------------------------// diff --git a/src/orange/orangeinp/ScaleUtils.cc b/src/orange/orangeinp/ScaleUtils.cc deleted file mode 100644 index a735cd38a3..0000000000 --- a/src/orange/orangeinp/ScaleUtils.cc +++ /dev/null @@ -1,32 +0,0 @@ -//------------------------------- -*- C++ -*- -------------------------------// -// Copyright Celeritas contributors: see top-level COPYRIGHT file for details -// SPDX-License-Identifier: (Apache-2.0 OR MIT) -//---------------------------------------------------------------------------// -//! \file orange/orangeinp/ScaleUtils.cc -//---------------------------------------------------------------------------// -#include "ScaleUtils.hh" - -#include "orange/orangeinp/CsgTree.hh" - -#include "detail/CsgLogicUtils.hh" - -namespace celeritas -{ -namespace orangeinp -{ -//---------------------------------------------------------------------------// -// Build postfix logic with original surface IDs intact -std::vector build_postfix_logic(CsgTree const& tree, NodeId n) -{ - using celeritas::orangeinp::detail::PostfixBuildLogicPolicy; - - PostfixBuildLogicPolicy const policy{tree}; - std::vector logic; - auto visitor = policy(logic); - visitor(n); - return logic; -} - -//---------------------------------------------------------------------------// -} // namespace orangeinp -} // namespace celeritas diff --git a/src/orange/orangeinp/ScaleUtils.hh b/src/orange/orangeinp/ScaleUtils.hh index f00a706380..e046c67047 100644 --- a/src/orange/orangeinp/ScaleUtils.hh +++ b/src/orange/orangeinp/ScaleUtils.hh @@ -4,28 +4,8 @@ //---------------------------------------------------------------------------// //! \file orange/orangeinp/ScaleUtils.hh //! \brief API compatibility functions for SCALE (https://scale.ornl.gov/) +//! \deprecated Remove in v1.0 //---------------------------------------------------------------------------// #pragma once -#include - -#include "CsgTypes.hh" - -#include "detail/NegatedSurfaceClipper.hh" - -namespace celeritas -{ -namespace orangeinp -{ -class CsgTree; - -//---------------------------------------------------------------------------// -// Build postfix logic with original surface IDs intact -std::vector build_postfix_logic(CsgTree const& tree, NodeId n); - -// NOTE: negated clipper must be accessible -using detail::NegatedSurfaceClipper; - -//---------------------------------------------------------------------------// -} // namespace orangeinp -} // namespace celeritas +#error "Deleted: update SCALE/Celeritas compatibility for v1.0" From ea5d7a47cb27e124d3f2f9baa1eb37b7b96fa080 Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Fri, 6 Feb 2026 06:06:34 -0500 Subject: [PATCH 13/24] Split build logic utils into header and cc --- src/orange/CMakeLists.txt | 1 + src/orange/detail/LogicUtils.cc | 8 +- src/orange/orangeinp/UnitProto.cc | 6 +- src/orange/orangeinp/detail/BuildLogic.cc | 277 ++++++++++++ src/orange/orangeinp/detail/BuildLogic.hh | 253 +++++++++++ src/orange/orangeinp/detail/CsgLogicUtils.hh | 440 ------------------- test/orange/orangeinp/CsgTreeUtils.test.cc | 2 +- 7 files changed, 538 insertions(+), 449 deletions(-) create mode 100644 src/orange/orangeinp/detail/BuildLogic.cc create mode 100644 src/orange/orangeinp/detail/BuildLogic.hh delete mode 100644 src/orange/orangeinp/detail/CsgLogicUtils.hh diff --git a/src/orange/CMakeLists.txt b/src/orange/CMakeLists.txt index 4465b41290..8d83790dcc 100644 --- a/src/orange/CMakeLists.txt +++ b/src/orange/CMakeLists.txt @@ -54,6 +54,7 @@ list(APPEND SOURCES orangeinp/Truncated.cc orangeinp/UnitProto.cc orangeinp/detail/BoundingZone.cc + orangeinp/detail/BuildLogic.cc orangeinp/detail/BuildIntersectRegion.cc orangeinp/detail/CsgUnitBuilder.cc orangeinp/detail/DeMorganSimplifier.cc diff --git a/src/orange/detail/LogicUtils.cc b/src/orange/detail/LogicUtils.cc index e03c11bdb9..33fb0f2708 100644 --- a/src/orange/detail/LogicUtils.cc +++ b/src/orange/detail/LogicUtils.cc @@ -11,7 +11,7 @@ #include "corecel/Assert.hh" #include "orange/orangeinp/CsgTree.hh" #include "orange/orangeinp/CsgTreeUtils.hh" -#include "orange/orangeinp/detail/CsgLogicUtils.hh" +#include "orange/orangeinp/detail/BuildLogic.hh" #include "../OrangeTypes.hh" @@ -99,11 +99,9 @@ simplify_negated_joins_postfix(Span postfix) NodeId root = tree.volumes().front(); // Convert simplified tree to postfix - orangeinp::detail::PostfixBuildLogicPolicy const policy{tree}; + orangeinp::detail::PostfixBuildLogicPolicy const make_builder{tree}; std::vector logic; - auto build_impl = policy(logic); - build_impl(root); - CELER_ENSURE(!logic.empty()); + make_builder(logic)(root); return logic; } diff --git a/src/orange/orangeinp/UnitProto.cc b/src/orange/orangeinp/UnitProto.cc index 6570645984..9430d585b4 100644 --- a/src/orange/orangeinp/UnitProto.cc +++ b/src/orange/orangeinp/UnitProto.cc @@ -35,7 +35,7 @@ #include "ObjectIO.json.hh" #include "Transformed.hh" -#include "detail/CsgLogicUtils.hh" +#include "detail/BuildLogic.hh" #include "detail/CsgUnit.hh" #include "detail/CsgUnitBuilder.hh" #include "detail/InternalSurfaceFlagger.hh" @@ -326,8 +326,8 @@ void UnitProto::build(ProtoBuilder& pb) const // Always use postfix logic for unit input, post-processing to // convert to tracking notation - detail::RuntimeBuildLogicPolicy const policy{ - LogicNotation::postfix, csg_unit.tree, sorted_local_surfaces}; + detail::DynamicBuildLogicPolicy const policy{ + LogicNotation::postfix, csg_unit.tree, &sorted_local_surfaces}; // Construct logic and faces with remapped surfaces for (auto vol_idx : range(unit_volumes.size())) { diff --git a/src/orange/orangeinp/detail/BuildLogic.cc b/src/orange/orangeinp/detail/BuildLogic.cc new file mode 100644 index 0000000000..9904b7ee02 --- /dev/null +++ b/src/orange/orangeinp/detail/BuildLogic.cc @@ -0,0 +1,277 @@ +//------------------------------- -*- C++ -*- -------------------------------// +// Copyright Celeritas contributors: see top-level COPYRIGHT file for details +// SPDX-License-Identifier: (Apache-2.0 OR MIT) +//---------------------------------------------------------------------------// +//! \file orange/orangeinp/detail/BuildLogic.cc +//---------------------------------------------------------------------------// +#include "BuildLogic.hh" + +#include + +#include "corecel/Assert.hh" +#include "corecel/math/Algorithms.hh" +#include "orange/OrangeTypes.hh" + +namespace celeritas +{ +namespace orangeinp +{ +namespace detail +{ +namespace +{ +//---------------------------------------------------------------------------// +/*! + * Sort the faces of a volume and remap the logic expression. + */ +BuildLogicResult::VecSurface remap_faces(BuildLogicResult::VecLogic& lgc) +{ + // Construct sorted vector of faces + BuildLogicResult::VecSurface faces; + for (auto const& v : lgc) + { + if (!logic::is_operator_token(v)) + { + faces.push_back(LocalSurfaceId{v}); + } + } + + // Sort and uniquify the vector + std::sort(faces.begin(), faces.end()); + faces.erase(std::unique(faces.begin(), faces.end()), faces.end()); + + // Remap logic + for (auto& v : lgc) + { + if (!logic::is_operator_token(v)) + { + auto iter + = find_sorted(faces.begin(), faces.end(), LocalSurfaceId{v}); + CELER_ASSUME(iter != faces.end()); + v = iter - faces.begin(); + } + } + return faces; +} + +} // namespace + +//---------------------------------------------------------------------------// +/*! + * Construct with optional mapping pointer. + * + * The optional surface mapping is an ordered vector of *existing* surface IDs. + * Those surface IDs will be replaced by the index in the array. All existing + * surface IDs must be present! + */ +template +BaseLogicBuilder::BaseLogicBuilder(CsgTree const& tree, + VecLogic& logic, + VecSurface const* vs) + : logic_{logic}, visit_node_{tree}, mapping_{vs} +{ +} + +//---------------------------------------------------------------------------// +/*! + * Build from a node ID. + */ +template +void BaseLogicBuilder::operator()(NodeId const& n) +{ + visit_node_(static_cast(*this), n); +} + +//---------------------------------------------------------------------------// +/*! + * Append the "true" token. + */ +template +void BaseLogicBuilder::operator()(True const&) +{ + logic_.push_back(logic::ltrue); +} + +//---------------------------------------------------------------------------// +/*! + * Explicit "False" should never be possible for a CSG cell. + * + * The 'false' standin is always aliased to "not true" in the CSG tree. + */ +template +void BaseLogicBuilder::operator()(False const&) +{ + CELER_ASSERT_UNREACHABLE(); +} + +//---------------------------------------------------------------------------// +/*! + * Push a surface ID. + */ +template +void BaseLogicBuilder::operator()(Surface const& s) +{ + CELER_EXPECT(s.id < logic::lbegin); + // Get index of original surface or remapped + logic_int sidx = [this, sid = s.id] { + if (!mapping_) + { + return sid.unchecked_get(); + } + else + { + // Remap by finding position of surface in our mapping + auto iter = find_sorted(mapping_->begin(), mapping_->end(), sid); + CELER_ASSERT(iter != mapping_->end()); + return logic_int(iter - mapping_->begin()); + } + }(); + + logic_.push_back(sidx); +} + +//---------------------------------------------------------------------------// +/*! + * Push an aliased node. + * + * Aliased node shouldn't be reachable if the tree is fully simplified, but + * could be reachable for testing purposes. + */ +template +void BaseLogicBuilder::operator()(Aliased const& n) +{ + (*this)(n.node); +} + +//---------------------------------------------------------------------------// +/*! + * Append negation after a node. + */ +void PostfixLogicBuilder::operator()(Negated const& n) +{ + (*this)(n.node); + logic_.push_back(logic::lnot); +} + +//---------------------------------------------------------------------------// +/*! + * Push multiply joined nodes. + */ +void PostfixLogicBuilder::operator()(Joined const& n) +{ + CELER_EXPECT(n.nodes.size() > 1); + + // Visit first node, then add conjunction for subsequent nodes + auto iter = n.nodes.begin(); + (*this)(*iter++); + + while (iter != n.nodes.end()) + { + (*this)(*iter++); + logic_.push_back(n.op); + } +} + +//---------------------------------------------------------------------------// +/*! + * Negate an expression. + */ +void InfixLogicBuilder::operator()(Negated const& n) +{ + logic_.push_back(logic::lnot); + (*this)(n.node); +} + +//---------------------------------------------------------------------------// +/*! + * Join a set of nodes. + */ +void InfixLogicBuilder::operator()(Joined const& n) +{ + CELER_EXPECT(n.nodes.size() > 1); + auto& logic = logic_; + logic.push_back(logic::lopen); + // Visit first node, then add conjunction for subsequent nodes + auto iter = n.nodes.begin(); + (*this)(*iter++); + + while (iter != n.nodes.end()) + { + logic.push_back(n.op); + (*this)(*iter++); + } + logic.push_back(logic::lclose); +} + +//---------------------------------------------------------------------------// +/*! + * Construct a logic representation of a node. + * + * The result is a pair of vectors: the sorted surface IDs comprising the faces + * of this volume, and the logical representation using \em face IDs, i.e. with + * the surfaces remapped to the index of the surface in the face vector. + * + * The function is templated on a policy class that determines the logic + * representation. The policy acts as a factory that creates a visitor to build + * the logic expression. + * + * The per-node local surfaces (faces) are sorted in ascending order of ID, not + * of access, since they're always evaluated sequentially rather than as part + * of the logic evaluation itself. + */ +template +BuildLogicResult build_logic(LogicPolicy const& policy, NodeId n) +{ + // static_assert(std::is_invocable_v); + + // Construct logic vector as local surface IDs + BuildLogicResult::VecLogic lgc; + auto build_impl = policy(lgc); + build_impl(n); + + return {remap_faces(lgc), std::move(lgc)}; +} + +//---------------------------------------------------------------------------// +/*! + * Construct with optional mapping. + */ +DynamicBuildLogicPolicy::DynamicBuildLogicPolicy(LogicNotation notation, + CsgTree const& tree, + VecSurface const* mapping) + : notation_{notation}, tree_{tree}, mapping_{mapping} +{ +} + +//---------------------------------------------------------------------------// +/*! + * Create a visitor for building logic. + */ +auto DynamicBuildLogicPolicy::operator()(VecLogic& logic) const -> Builder +{ + CELER_EXPECT(logic.empty()); + + switch (notation_) + { + case LogicNotation::postfix: + return PostfixLogicBuilder{tree_, logic, mapping_}; + case LogicNotation::infix: + return InfixLogicBuilder{tree_, logic, mapping_}; + default: + CELER_ASSERT_UNREACHABLE(); + } +} + +//---------------------------------------------------------------------------// +// TEMPLATE INSTANTIATION +//---------------------------------------------------------------------------// + +template BuildLogicResult build_logic(PostfixBuildLogicPolicy const&, NodeId); +template BuildLogicResult build_logic(InfixBuildLogicPolicy const&, NodeId); +template BuildLogicResult build_logic(DynamicBuildLogicPolicy const&, NodeId); + +//---------------------------------------------------------------------------// +} // namespace detail +} // namespace orangeinp +} // namespace celeritas diff --git a/src/orange/orangeinp/detail/BuildLogic.hh b/src/orange/orangeinp/detail/BuildLogic.hh new file mode 100644 index 0000000000..710d840d3e --- /dev/null +++ b/src/orange/orangeinp/detail/BuildLogic.hh @@ -0,0 +1,253 @@ +//------------------------------- -*- C++ -*- -------------------------------// +// Copyright Celeritas contributors: see top-level COPYRIGHT file for details +// SPDX-License-Identifier: (Apache-2.0 OR MIT) +//---------------------------------------------------------------------------// +//! \file orange/orangeinp/detail/BuildLogic.hh +//---------------------------------------------------------------------------// +#pragma once + +#include +#include +#include + +#include "corecel/cont/VariantUtils.hh" +#include "orange/OrangeTypes.hh" +#include "orange/orangeinp/CsgTree.hh" +#include "orange/orangeinp/CsgTypes.hh" + +namespace celeritas +{ +namespace orangeinp +{ +namespace detail +{ +//---------------------------------------------------------------------------// +/*! + * Result of building a logic representation of a node. + */ +struct BuildLogicResult +{ + using VecLogic = std::vector; + using VecSurface = std::vector; + + VecSurface faces; + VecLogic logic; +}; + +//---------------------------------------------------------------------------// +/*! + * Construct a logic representation of a node. + * + * The result is a pair of vectors: the sorted surface IDs comprising the faces + * of this volume, and the logical representation using \em face IDs, i.e. with + * the surfaces remapped to the index of the surface in the face vector. + * + * The function is templated on a policy class that determines the logic + * representation. The policy acts as a factory that creates a visitor to build + * the logic expression. + * + * The per-node local surfaces (faces) are sorted in ascending order of ID, not + * of access, since they're always evaluated sequentially rather than as part + * of the logic evaluation itself. + */ +template +BuildLogicResult build_logic(LogicPolicy const& policy, NodeId n); + +//---------------------------------------------------------------------------// +// BUILDERS +//---------------------------------------------------------------------------// +/*! + * Base class for logic builder visitors following CRTP pattern. + * + * Visitors recursively traverse the CSG tree and append to a logic vector. + * The call operator for Negated and Joined are not implemented in the base + * visitor and must be provided by the derived class. + */ +template +class BaseLogicBuilder +{ + public: + //!@{ + //! \name Type aliases + using VecLogic = std::vector; + using VecSurface = std::vector; + //!@} + + static_assert(std::is_same_v, + "unsupported: add enum logic conversion for different-sized " + "face and surface ints"); + + public: + // Construct with optional mapping pointer + BaseLogicBuilder(CsgTree const& tree, + VecLogic& logic, + VecSurface const* vs = nullptr); + //! Build from a node ID + void operator()(NodeId const& n); + + //!@{ + //! \name Visit a node directly + // Append 'true' + void operator()(True const&); + // False is never explicitly part of the node tree + void operator()(False const&); + // Append a surface ID + void operator()(Surface const&); + // Aliased nodes should never be reachable explicitly + void operator()(Aliased const&); + //!@} + + protected: + VecLogic& logic_; + + private: + ContainerVisitor visit_node_; + VecSurface const* mapping_{nullptr}; +}; + +//---------------------------------------------------------------------------// +/*! + * Visitor for constructing logic in postfix notation. + * + * Example: \verbatim + all(1, 3, 5) -> "0 1 & 2 & &" + all(1, 3, !all(2, 4)) -> "0 2 & 1 3 & ~ &" + * \endverbatim + */ +class PostfixLogicBuilder : public BaseLogicBuilder +{ + public: + using BaseLogicBuilder::BaseLogicBuilder; + + //!@{ + //! \name Visit a node directly + using BaseLogicBuilder::operator(); + + // Visit a negated node and append 'not'. + void operator()(Negated const& n); + + // Visit daughter nodes and append the conjunction. + void operator()(Joined const& n); + //!@} +}; + +//---------------------------------------------------------------------------// +/*! + * Visitor for constructing logic in infix notation. + * + * Example: \verbatim + all(1, 3, 5) -> "(0 & 1 & 2)" + all(1, 3, any(~(2), ~(4))) -> "(0 & 2 & (~1 | ~3))" + * \endverbatim + */ +class InfixLogicBuilder : public BaseLogicBuilder +{ + public: + using BaseLogicBuilder::BaseLogicBuilder; + + //!@{ + //! \name Visit a node directly + using BaseLogicBuilder::operator(); + + // Append 'not' and visit a negated node. + void operator()(Negated const& n); + + // Visit daughter nodes and append the conjunction. + void operator()(Joined const& n); + //!@} +}; + +//---------------------------------------------------------------------------// +// POLICIES +//---------------------------------------------------------------------------// +/*! + * Policy for building logic expressions. + * \tparam LogicBuilder Visitor that constructs an input logic vector. + * + * This immutable factory creates visitors that construct logic expressions + * from node IDs. It can be passed by const reference to \c build_logic. + */ +template +class StaticBuildLogicPolicy +{ + static_assert(std::is_invocable_v); + + public: + //!@{ + //! \name Type aliases + using VecLogic = std::vector; + using VecSurface = std::vector; + //!@} + + public: + // Construct without mapping + explicit StaticBuildLogicPolicy(CsgTree const& tree) : tree_{tree} {} + // Construct with mapping + StaticBuildLogicPolicy(CsgTree const& tree, VecSurface const& vs) + : tree_{tree}, mapping_{&vs} + { + } + + //! Create a visitor for building logic + auto operator()(VecLogic& logic) const + { + return LogicBuilder{tree_, logic, mapping_}; + } + + private: + CsgTree const& tree_; + VecSurface const* mapping_{nullptr}; +}; + +//---------------------------------------------------------------------------// + +using PostfixBuildLogicPolicy = StaticBuildLogicPolicy; +using InfixBuildLogicPolicy = StaticBuildLogicPolicy; + +//---------------------------------------------------------------------------// +/*! + * Runtime-dispatching policy for building logic expressions. + * + * This policy class selects between postfix and infix notation at runtime + * based on the input \c LogicNotation enum value. + */ +class DynamicBuildLogicPolicy +{ + public: + //!@{ + //! \name Type aliases + using VecLogic = std::vector; + using VecSurface = std::vector; + using Builder = std::function; + //!@} + + public: + // Construct with optional mapping + DynamicBuildLogicPolicy(LogicNotation notation, + CsgTree const& tree, + VecSurface const* mapping); + + // Create a visitor for building logic + Builder operator()(VecLogic& logic) const; + + private: + LogicNotation notation_; + CsgTree const& tree_; + VecSurface const* mapping_{nullptr}; +}; + +//---------------------------------------------------------------------------// +// TEMPLATE INSTANTIATION +//---------------------------------------------------------------------------// + +extern template BuildLogicResult +build_logic(PostfixBuildLogicPolicy const&, NodeId); +extern template BuildLogicResult +build_logic(InfixBuildLogicPolicy const&, NodeId); +extern template BuildLogicResult +build_logic(DynamicBuildLogicPolicy const&, NodeId); + +//---------------------------------------------------------------------------// +} // namespace detail +} // namespace orangeinp +} // namespace celeritas diff --git a/src/orange/orangeinp/detail/CsgLogicUtils.hh b/src/orange/orangeinp/detail/CsgLogicUtils.hh deleted file mode 100644 index f1ea081709..0000000000 --- a/src/orange/orangeinp/detail/CsgLogicUtils.hh +++ /dev/null @@ -1,440 +0,0 @@ -//------------------------------- -*- C++ -*- -------------------------------// -// Copyright Celeritas contributors: see top-level COPYRIGHT file for details -// SPDX-License-Identifier: (Apache-2.0 OR MIT) -//---------------------------------------------------------------------------// -//! \file orange/orangeinp/detail/CsgLogicUtils.hh -//---------------------------------------------------------------------------// -#pragma once - -#include -#include -#include -#include - -#include "corecel/Assert.hh" -#include "corecel/cont/VariantUtils.hh" -#include "corecel/math/Algorithms.hh" -#include "orange/OrangeTypes.hh" - -#include "../CsgTree.hh" -#include "../CsgTypes.hh" - -namespace celeritas -{ -namespace orangeinp -{ -namespace detail -{ -//---------------------------------------------------------------------------// -/*! - * Result of building a logic representation of a node. - */ -struct BuildLogicResult -{ - using VecLogic = std::vector; - using VecSurface = std::vector; - - VecSurface faces; - VecLogic logic; -}; - -//---------------------------------------------------------------------------// -/*! - * Sort the faces of a volume and remap the logic expression. - */ -inline BuildLogicResult::VecSurface remap_faces(BuildLogicResult::VecLogic& lgc) -{ - // Construct sorted vector of faces - BuildLogicResult::VecSurface faces; - for (auto const& v : lgc) - { - if (!logic::is_operator_token(v)) - { - faces.push_back(LocalSurfaceId{v}); - } - } - - // Sort and uniquify the vector - std::sort(faces.begin(), faces.end()); - faces.erase(std::unique(faces.begin(), faces.end()), faces.end()); - - // Remap logic - for (auto& v : lgc) - { - if (!logic::is_operator_token(v)) - { - auto iter - = find_sorted(faces.begin(), faces.end(), LocalSurfaceId{v}); - CELER_ASSUME(iter != faces.end()); - v = iter - faces.begin(); - } - } - return faces; -} - -//---------------------------------------------------------------------------// -/*! - * Base class for logic builder visitors following CRTP pattern. - * - * Visitors recursively traverse the CSG tree and append to a logic vector. - * The call operator for Negated and Joined are not implemented in the base - * visitor and must be provided by the derived class. - */ -template -class BaseLogicBuilder -{ - public: - //!@{ - //! \name Type aliases - using VecLogic = std::vector; - using VecSurface = std::vector; - //!@} - - static_assert(std::is_same_v, - "unsupported: add enum logic conversion for different-sized " - "face and surface ints"); - - public: - // Construct with optional mapping pointer - inline BaseLogicBuilder(CsgTree const& tree, - VecLogic& logic, - VecSurface const* vs = nullptr); - //! Build from a node ID - inline void operator()(NodeId const& n); - - //!@{ - //! \name Visit a node directly - // Append 'true' - inline void operator()(True const&); - // False is never explicitly part of the node tree - inline void operator()(False const&); - // Append a surface ID - inline void operator()(Surface const&); - // Aliased nodes should never be reachable explicitly - inline void operator()(Aliased const&); - //!@} - - protected: - VecLogic& logic_; - - private: - ContainerVisitor visit_node_; - VecSurface const* mapping_{nullptr}; -}; - -//---------------------------------------------------------------------------// -/*! - * Construct with optional mapping pointer. - * - * The optional surface mapping is an ordered vector of *existing* surface IDs. - * Those surface IDs will be replaced by the index in the array. All existing - * surface IDs must be present! - */ -template -BaseLogicBuilder::BaseLogicBuilder(CsgTree const& tree, - VecLogic& logic, - VecSurface const* vs) - : logic_{logic}, visit_node_{tree}, mapping_{vs} -{ -} - -//---------------------------------------------------------------------------// -/*! - * Build from a node ID. - */ -template -void BaseLogicBuilder::operator()(NodeId const& n) -{ - visit_node_(static_cast(*this), n); -} - -//---------------------------------------------------------------------------// -/*! - * Append the "true" token. - */ -template -void BaseLogicBuilder::operator()(True const&) -{ - logic_.push_back(logic::ltrue); -} - -//---------------------------------------------------------------------------// -/*! - * Explicit "False" should never be possible for a CSG cell. - * - * The 'false' standin is always aliased to "not true" in the CSG tree. - */ -template -void BaseLogicBuilder::operator()(False const&) -{ - CELER_ASSERT_UNREACHABLE(); -} - -//---------------------------------------------------------------------------// -/*! - * Push a surface ID. - */ -template -void BaseLogicBuilder::operator()(Surface const& s) -{ - CELER_EXPECT(s.id < logic::lbegin); - // Get index of original surface or remapped - logic_int sidx = [this, sid = s.id] { - if (!mapping_) - { - return sid.unchecked_get(); - } - else - { - // Remap by finding position of surface in our mapping - auto iter = find_sorted(mapping_->begin(), mapping_->end(), sid); - CELER_ASSERT(iter != mapping_->end()); - return logic_int(iter - mapping_->begin()); - } - }(); - - logic_.push_back(sidx); -} - -//---------------------------------------------------------------------------// -/*! - * Push an aliased node. - * - * Aliased node shouldn't be reachable if the tree is fully simplified, but - * could be reachable for testing purposes. - */ -template -void BaseLogicBuilder::operator()(Aliased const& n) -{ - (*this)(n.node); -} - -//---------------------------------------------------------------------------// -/*! - * Visitor for constructing logic in postfix notation. - * - * Example: \verbatim - all(1, 3, 5) -> "0 1 & 2 & &" - all(1, 3, !all(2, 4)) -> "0 2 & 1 3 & ~ &" - * \endverbatim - */ -class PostfixLogicBuilder : public BaseLogicBuilder -{ - public: - using BaseLogicBuilder::BaseLogicBuilder; - - //!@{ - //! \name Visit a node directly - using BaseLogicBuilder::operator(); - - //! Visit a negated node and append 'not'. - void operator()(Negated const& n) - { - (*this)(n.node); - logic_.push_back(logic::lnot); - } - - //! Visit daughter nodes and append the conjunction. - void operator()(Joined const& n) - { - CELER_EXPECT(n.nodes.size() > 1); - - // Visit first node, then add conjunction for subsequent nodes - auto iter = n.nodes.begin(); - (*this)(*iter++); - - while (iter != n.nodes.end()) - { - (*this)(*iter++); - logic_.push_back(n.op); - } - } - //!@} -}; - -//---------------------------------------------------------------------------// -/*! - * Visitor for constructing logic in infix notation. - * - * Example: \verbatim - all(1, 3, 5) -> "(0 & 1 & 2)" - all(1, 3, any(~(2), ~(4))) -> "(0 & 2 & (~1 | ~3))" - * \endverbatim - */ -class InfixLogicBuilder : public BaseLogicBuilder -{ - public: - using BaseLogicBuilder::BaseLogicBuilder; - - //!@{ - //! \name Visit a node directly - using BaseLogicBuilder::operator(); - - //! Append 'not' and visit a negated node. - void operator()(Negated const& n) - { - logic_.push_back(logic::lnot); - (*this)(n.node); - } - - //! Visit daughter nodes and append the conjunction. - void operator()(Joined const& n) - { - CELER_EXPECT(n.nodes.size() > 1); - auto& logic = logic_; - logic.push_back(logic::lopen); - // Visit first node, then add conjunction for subsequent nodes - auto iter = n.nodes.begin(); - (*this)(*iter++); - - while (iter != n.nodes.end()) - { - logic.push_back(n.op); - (*this)(*iter++); - } - logic.push_back(logic::lclose); - } - //!@} -}; - -//---------------------------------------------------------------------------// -/*! - * Construct a logic representation of a node. - * - * The result is a pair of vectors: the sorted surface IDs comprising the faces - * of this volume, and the logical representation using \em face IDs, i.e. with - * the surfaces remapped to the index of the surface in the face vector. - * - * The function is templated on a policy class that determines the logic - * representation. The policy acts as a factory that creates a visitor to build - * the logic expression. - * - * The per-node local surfaces (faces) are sorted in ascending order of ID, not - * of access, since they're always evaluated sequentially rather than as part - * of the logic evaluation itself. - */ -template -inline BuildLogicResult build_logic(LogicPolicy const& policy, NodeId n) -{ - // Construct logic vector as local surface IDs - BuildLogicResult::VecLogic lgc; - auto visitor = policy(lgc); - - // Handle both direct builders and variant-wrapped builders - if constexpr (std::is_same_v< - decltype(visitor), - std::variant>) - { - std::visit([n](auto& v) { v(n); }, visitor); - } - else - { - visitor(n); - } - - return {remap_faces(lgc), std::move(lgc)}; -} - -//---------------------------------------------------------------------------// -/*! - * Policy for building logic expressions. - * - * This immutable factory creates visitors that construct logic expressions. - * It can be passed by const reference to \c build_logic. - * - * \tparam LogicBuilder The builder type (PostfixLogicBuilder or - * InfixLogicBuilder) - */ -template -class BuildLogicPolicy -{ - public: - //!@{ - //! \name Type aliases - using VecLogic = std::vector; - using VecSurface = std::vector; - //!@} - - public: - // Construct without mapping - explicit BuildLogicPolicy(CsgTree const& tree) : tree_{tree} {} - // Construct with mapping - BuildLogicPolicy(CsgTree const& tree, VecSurface const& vs) - : tree_{tree}, mapping_{&vs} - { - } - - //! Create a visitor for building logic - auto operator()(VecLogic& logic) const - { - return LogicBuilder{tree_, logic, mapping_}; - } - - private: - CsgTree const& tree_; - VecSurface const* mapping_{nullptr}; -}; - -//---------------------------------------------------------------------------// -/*! - * Policy classes are factories. - */ -using PostfixBuildLogicPolicy = BuildLogicPolicy; -using InfixBuildLogicPolicy = BuildLogicPolicy; - -//---------------------------------------------------------------------------// -/*! - * Runtime-dispatching policy for building logic expressions. - * - * This policy class selects between postfix and infix notation at runtime - * based on a LogicNotation enum value. The operator() returns a variant - * containing the appropriate builder type. - */ -class RuntimeBuildLogicPolicy -{ - public: - //!@{ - //! \name Type aliases - using VecLogic = std::vector; - using VecSurface = std::vector; - using Builder = std::variant; - //!@} - - public: - // Construct without mapping - RuntimeBuildLogicPolicy(LogicNotation notation, CsgTree const& tree) - : notation_{notation}, tree_{tree} - { - } - // Construct with mapping - RuntimeBuildLogicPolicy(LogicNotation notation, - CsgTree const& tree, - VecSurface const& vs) - : notation_{notation}, tree_{tree}, mapping_{&vs} - { - } - - //! Create a visitor for building logic - Builder operator()(VecLogic& logic) const - { - switch (notation_) - { - case LogicNotation::postfix: - return PostfixLogicBuilder{tree_, logic, mapping_}; - case LogicNotation::infix: - return InfixLogicBuilder{tree_, logic, mapping_}; - default: - CELER_ASSERT_UNREACHABLE(); - } - } - - private: - LogicNotation notation_; - CsgTree const& tree_; - VecSurface const* mapping_{nullptr}; -}; - -//---------------------------------------------------------------------------// -} // namespace detail -} // namespace orangeinp -} // namespace celeritas diff --git a/test/orange/orangeinp/CsgTreeUtils.test.cc b/test/orange/orangeinp/CsgTreeUtils.test.cc index 19f9c292a2..c87f28407a 100644 --- a/test/orange/orangeinp/CsgTreeUtils.test.cc +++ b/test/orange/orangeinp/CsgTreeUtils.test.cc @@ -10,7 +10,7 @@ #include "orange/detail/LogicUtils.hh" #include "orange/orangeinp/CsgTree.hh" #include "orange/orangeinp/CsgTypes.hh" -#include "orange/orangeinp/detail/CsgLogicUtils.hh" +#include "orange/orangeinp/detail/BuildLogic.hh" #include "orange/orangeinp/detail/InternalSurfaceFlagger.hh" #include "orange/orangeinp/detail/SenseEvaluator.hh" #include "orange/surf/VariantSurface.hh" From 8c8c59d5e0b3554e6be5003d40c9434bfda1ce11 Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Fri, 6 Feb 2026 06:42:54 -0500 Subject: [PATCH 14/24] Split logic conversion and IO utilities --- src/orange/CMakeLists.txt | 3 +- src/orange/OrangeInputIO.json.cc | 2 +- src/orange/OrangeParams.cc | 2 +- .../detail/{LogicUtils.cc => ConvertLogic.cc} | 86 +----------- src/orange/detail/ConvertLogic.hh | 32 +++++ src/orange/detail/LogicIO.cc | 125 ++++++++++++++++++ src/orange/detail/LogicIO.hh | 31 +++++ src/orange/detail/LogicUtils.hh | 79 ----------- test/orange/detail/LogicUtils.test.cc | 23 +--- test/orange/orangeinp/CsgTreeUtils.test.cc | 2 +- .../orange/univ/detail/InfixEvaluator.test.cc | 5 +- .../orange/univ/detail/LogicEvaluator.test.cc | 3 +- 12 files changed, 206 insertions(+), 187 deletions(-) rename src/orange/detail/{LogicUtils.cc => ConvertLogic.cc} (85%) create mode 100644 src/orange/detail/ConvertLogic.hh create mode 100644 src/orange/detail/LogicIO.cc create mode 100644 src/orange/detail/LogicIO.hh delete mode 100644 src/orange/detail/LogicUtils.hh diff --git a/src/orange/CMakeLists.txt b/src/orange/CMakeLists.txt index 8d83790dcc..73d8c504f9 100644 --- a/src/orange/CMakeLists.txt +++ b/src/orange/CMakeLists.txt @@ -25,7 +25,8 @@ list(APPEND SOURCES detail/BIHBuilder.cc detail/BIHPartitioner.cc detail/DepthCalculator.cc - detail/LogicUtils.cc + detail/ConvertLogic.cc + detail/LogicIO.cc detail/OrangeInputIOImpl.json.cc detail/RectArrayInserter.cc detail/SurfacesRecordBuilder.cc diff --git a/src/orange/OrangeInputIO.json.cc b/src/orange/OrangeInputIO.json.cc index 162248a2e4..00eb13d095 100644 --- a/src/orange/OrangeInputIO.json.cc +++ b/src/orange/OrangeInputIO.json.cc @@ -29,7 +29,7 @@ #include "OrangeTypes.hh" #include "OrangeTypesIO.json.hh" // IWYU pragma: keep -#include "detail/LogicUtils.hh" +#include "detail/LogicIO.hh" #include "detail/OrangeInputIOImpl.json.hh" namespace celeritas diff --git a/src/orange/OrangeParams.cc b/src/orange/OrangeParams.cc index 6c04b35d9d..71479a1777 100644 --- a/src/orange/OrangeParams.cc +++ b/src/orange/OrangeParams.cc @@ -24,7 +24,6 @@ #include "geocel/BoundingBox.hh" #include "geocel/GeantGeoParams.hh" #include "geocel/VolumeParams.hh" -#include "orange/detail/LogicUtils.hh" #include "OrangeData.hh" // IWYU pragma: associated #include "OrangeInput.hh" @@ -33,6 +32,7 @@ #include "g4org/Converter.hh" #include "univ/detail/LogicStack.hh" +#include "detail/ConvertLogic.hh" #include "detail/DepthCalculator.hh" #include "detail/RectArrayInserter.hh" #include "detail/UnitInserter.hh" diff --git a/src/orange/detail/LogicUtils.cc b/src/orange/detail/ConvertLogic.cc similarity index 85% rename from src/orange/detail/LogicUtils.cc rename to src/orange/detail/ConvertLogic.cc index 33fb0f2708..a10425b6a9 100644 --- a/src/orange/detail/LogicUtils.cc +++ b/src/orange/detail/ConvertLogic.cc @@ -2,9 +2,9 @@ // Copyright Celeritas contributors: see top-level COPYRIGHT file for details // SPDX-License-Identifier: (Apache-2.0 OR MIT) //---------------------------------------------------------------------------// -//! \file orange/detail/LogicUtils.cc +//! \file orange/detail/ConvertLogic.cc //---------------------------------------------------------------------------// -#include "LogicUtils.hh" +#include "ConvertLogic.hh" #include @@ -137,7 +137,7 @@ inline int precedence(logic_int token) /*! * Return true if the operator is right associative. */ -inline bool is_right_associative(logic_int token) +constexpr bool is_right_associative(logic_int token) { return token == logic::lnot; } @@ -321,6 +321,8 @@ class PostfixStack std::vector postfix_; std::vector operators_; }; + +//---------------------------------------------------------------------------// } // namespace //---------------------------------------------------------------------------// @@ -424,84 +426,6 @@ std::vector convert_to_postfix(Span infix) return std::move(postfix).get_postfix(); } -//---------------------------------------------------------------------------// -/*! - * Build a logic definition from a C string. - * - * A valid string satisfies the regex "[0-9~!| ()]+", but the result may - * not be a valid logic expression. (The volume inserter will ensure that the - * logic expression at least is consistent for a CSG region definition.) - * - * Example: - * \code - - parse_logic("4 ~ 5 & 6 &"); - - \endcode - */ -VecLogic string_to_logic(std::string_view s) -{ - VecLogic result; - - logic_int surf_id{}; - bool reading_surf{false}; - int parens_depth{0}; - for (char v : s) - { - if (v >= '0' && v <= '9') - { - // Parse a surface number. 'Push' this digit onto the surface ID by - // multiplying the existing ID by 10. - if (!reading_surf) - { - surf_id = 0; - reading_surf = true; - } - surf_id = 10 * surf_id + (v - '0'); - continue; - } - else if (reading_surf) - { - // Next char is end of word or end of string - result.push_back(surf_id); - reading_surf = false; - } - - // Parse a logic token - // NOLINTNEXTLINE(bugprone-switch-missing-default-case) - switch (v) - { - case '(': - result.push_back(logic::lopen); - ++parens_depth; - continue; - case ')': - CELER_VALIDATE(parens_depth > 0, - << "unmatched ')' in logic string"); - --parens_depth; - result.push_back(logic::lclose); - continue; - // clang-format off - case '|': result.push_back(logic::lor); continue; - case '&': result.push_back(logic::land); continue; - case '~': result.push_back(logic::lnot); continue; - case '*': result.push_back(logic::ltrue); continue; - // clang-format on - } - CELER_VALIDATE(v == ' ', - << "unexpected token '" << v - << "' while parsing logic string"); - } - if (reading_surf) - { - result.push_back(surf_id); - } - - CELER_VALIDATE(parens_depth == 0, << "unmatched '(' in logic string"); - - return result; -} - //---------------------------------------------------------------------------// /*! * Convert logic expressions in an OrangeInput to the desired notation. diff --git a/src/orange/detail/ConvertLogic.hh b/src/orange/detail/ConvertLogic.hh new file mode 100644 index 0000000000..c981816a5b --- /dev/null +++ b/src/orange/detail/ConvertLogic.hh @@ -0,0 +1,32 @@ +//------------------------------- -*- C++ -*- -------------------------------// +// Copyright Celeritas contributors: see top-level COPYRIGHT file for details +// SPDX-License-Identifier: (Apache-2.0 OR MIT) +//---------------------------------------------------------------------------// +//! \file orange/detail/LogicUtils.hh +//---------------------------------------------------------------------------// +#pragma once + +#include + +#include "corecel/cont/Span.hh" +#include "orange/OrangeInput.hh" + +#include "../OrangeTypes.hh" + +namespace celeritas +{ +namespace detail +{ +//---------------------------------------------------------------------------// +// Convert a postfix logic expression to an infix expression. +std::vector convert_to_infix(Span postfix); + +// Convert an infix logic expression to a postfix expression. +std::vector convert_to_postfix(Span infix); + +// Convert logic expressions in an OrangeInput to the desired notation +void convert_logic(OrangeInput& input, LogicNotation to); + +//---------------------------------------------------------------------------// +} // namespace detail +} // namespace celeritas diff --git a/src/orange/detail/LogicIO.cc b/src/orange/detail/LogicIO.cc new file mode 100644 index 0000000000..4b0ed75cdd --- /dev/null +++ b/src/orange/detail/LogicIO.cc @@ -0,0 +1,125 @@ +//------------------------------- -*- C++ -*- -------------------------------// +// Copyright Celeritas contributors: see top-level COPYRIGHT file for details +// SPDX-License-Identifier: (Apache-2.0 OR MIT) +//---------------------------------------------------------------------------// +//! \file orange/detail/LogicIO.cc +//---------------------------------------------------------------------------// +#include "LogicIO.hh" + +#include + +#include "corecel/Assert.hh" +#include "corecel/io/Join.hh" + +namespace celeritas +{ +namespace detail +{ +//---------------------------------------------------------------------------// +/*! + * Convert a logic token to a string. + */ +void logic_to_stream(std::ostream& os, logic_int val) +{ + if (logic::is_operator_token(val)) + { + os << to_char(static_cast(val)); + } + else + { + // Just a face ID + os << val; + } +} + +//---------------------------------------------------------------------------// +/*! + * Convert a logic vector to a string. + */ +std::string logic_to_string(std::vector const& logic) +{ + return to_string(celeritas::join_stream( + logic.begin(), logic.end(), ' ', logic_to_stream)); +} + +//---------------------------------------------------------------------------// +/*! + * Build a logic definition from a string. + * + * A valid string satisfies the regex "[0-9~!| ()]+", and must have balanced + * parentheses, but the result may not be a valid logic expression and its + * interpretation depends on the logic notation. + * + * \par Example + * \code + + string_to_logic("4 ~ 5 & 6 &"); + + \endcode + */ +std::vector string_to_logic(std::string_view s) +{ + std::vector result; + + logic_int surf_id{}; + bool reading_surf{false}; + int parens_depth{0}; + for (char v : s) + { + if (v >= '0' && v <= '9') + { + // Parse a surface number. 'Push' this digit onto the surface ID by + // multiplying the existing ID by 10. + if (!reading_surf) + { + surf_id = 0; + reading_surf = true; + } + surf_id = 10 * surf_id + (v - '0'); + continue; + } + else if (reading_surf) + { + // Next char is end of word or end of string + result.push_back(surf_id); + reading_surf = false; + } + + // Parse a logic token + // NOLINTNEXTLINE(bugprone-switch-missing-default-case) + switch (v) + { + case '(': + result.push_back(logic::lopen); + ++parens_depth; + continue; + case ')': + CELER_VALIDATE(parens_depth > 0, + << "unmatched ')' in logic string"); + --parens_depth; + result.push_back(logic::lclose); + continue; + // clang-format off + case '|': result.push_back(logic::lor); continue; + case '&': result.push_back(logic::land); continue; + case '~': result.push_back(logic::lnot); continue; + case '*': result.push_back(logic::ltrue); continue; + // clang-format on + } + CELER_VALIDATE(v == ' ', + << "unexpected token '" << v + << "' while parsing logic string"); + } + if (reading_surf) + { + result.push_back(surf_id); + } + + CELER_VALIDATE(parens_depth == 0, << "unmatched '(' in logic string"); + + return result; +} + +//---------------------------------------------------------------------------// +} // namespace detail +} // namespace celeritas diff --git a/src/orange/detail/LogicIO.hh b/src/orange/detail/LogicIO.hh new file mode 100644 index 0000000000..ccb2fc60ed --- /dev/null +++ b/src/orange/detail/LogicIO.hh @@ -0,0 +1,31 @@ +//------------------------------- -*- C++ -*- -------------------------------// +// Copyright Celeritas contributors: see top-level COPYRIGHT file for details +// SPDX-License-Identifier: (Apache-2.0 OR MIT) +//---------------------------------------------------------------------------// +//! \file orange/detail/LogicIO.hh +//---------------------------------------------------------------------------// +#pragma once + +#include +#include +#include + +#include "orange/OrangeTypes.hh" + +namespace celeritas +{ +namespace detail +{ +//---------------------------------------------------------------------------// +// Stream a logic token +void logic_to_stream(std::ostream& os, logic_int val); + +// Convert a logic vector to a string. +std::string logic_to_string(std::vector const& logic); + +// Build a logic definition from a string +std::vector string_to_logic(std::string_view s); + +//---------------------------------------------------------------------------// +} // namespace detail +} // namespace celeritas diff --git a/src/orange/detail/LogicUtils.hh b/src/orange/detail/LogicUtils.hh deleted file mode 100644 index 60db6f453d..0000000000 --- a/src/orange/detail/LogicUtils.hh +++ /dev/null @@ -1,79 +0,0 @@ -//------------------------------- -*- C++ -*- -------------------------------// -// Copyright Celeritas contributors: see top-level COPYRIGHT file for details -// SPDX-License-Identifier: (Apache-2.0 OR MIT) -//---------------------------------------------------------------------------// -//! \file orange/detail/LogicUtils.hh -//---------------------------------------------------------------------------// -#pragma once - -#include -#include -#include - -#include "corecel/Assert.hh" -#include "corecel/cont/Span.hh" -#include "corecel/io/Join.hh" -#include "orange/OrangeInput.hh" - -#include "../OrangeTypes.hh" - -namespace celeritas -{ -namespace detail -{ -//---------------------------------------------------------------------------// -using VecLogic = std::vector; - -//---------------------------------------------------------------------------// -/*! - * Convert a logic token to a string. - */ -inline void logic_to_stream(std::ostream& os, logic_int val) -{ - if (logic::is_operator_token(val)) - { - os << to_char(static_cast(val)); - } - else - { - // Just a face ID - os << val; - } -} - -//---------------------------------------------------------------------------// -/*! - * Convert a logic vector to a string. - */ -inline std::string logic_to_string(std::vector const& logic) -{ - return to_string(celeritas::join_stream( - logic.begin(), logic.end(), ' ', logic_to_stream)); -} - -//---------------------------------------------------------------------------// -/*! - * Convert a postfix logic expression to an infix expression. - */ -std::vector convert_to_infix(Span postfix); - -/*! - * Convert an infix logic expression to a postfix expression. - */ -std::vector convert_to_postfix(Span infix); - -//---------------------------------------------------------------------------// -/*! - * Build a logic definition from a C string. - */ -std::vector string_to_logic(std::string_view s); - -//---------------------------------------------------------------------------// -/*! - * Convert logic expressions in an OrangeInput to the desired notation. - */ -void convert_logic(OrangeInput& input, LogicNotation to); - -//---------------------------------------------------------------------------// -} // namespace detail -} // namespace celeritas diff --git a/test/orange/detail/LogicUtils.test.cc b/test/orange/detail/LogicUtils.test.cc index 1566c2bc8e..0d2b1c7239 100644 --- a/test/orange/detail/LogicUtils.test.cc +++ b/test/orange/detail/LogicUtils.test.cc @@ -5,14 +5,13 @@ //! \file orange/detail/LogicUtils.test.cc //---------------------------------------------------------------------------// -#include "orange/detail/LogicUtils.hh" - -#include #include #include #include "corecel/cont/Span.hh" #include "orange/OrangeTypes.hh" +#include "orange/detail/ConvertLogic.hh" +#include "orange/detail/LogicIO.hh" #include "celeritas_test.hh" @@ -24,17 +23,6 @@ namespace test { //---------------------------------------------------------------------------// -std::vector postfix_to_infix(std::string_view postfix) -{ - auto postfix_expr = string_to_logic(postfix.data()); - return convert_to_infix(make_span(postfix_expr)); -} - -std::vector infix_to_postfix(std::vector infix) -{ - return convert_to_postfix(make_span(infix)); -} - OrangeInput make_input_with_logic(std::vector logic, LogicNotation notation) { @@ -53,10 +41,11 @@ make_input_with_logic(std::vector logic, LogicNotation notation) TEST(NotationConverter, basic) { auto round_trip = [](std::string_view postfix, std::string_view infix) { - auto infix_expr = postfix_to_infix(postfix); + auto postfix_expr = string_to_logic(postfix); + auto infix_expr = convert_to_infix(make_span(postfix_expr)); EXPECT_EQ(logic_to_string(infix_expr), infix); - auto postfix_expr = infix_to_postfix(infix_expr); - EXPECT_EQ(logic_to_string(postfix_expr), postfix); + auto new_postfix_expr = convert_to_postfix(make_span(infix_expr)); + EXPECT_EQ(logic_to_string(new_postfix_expr), postfix); }; round_trip("0 1 ~ & 2 & 3 ~ & 4 & 5 ~ & ~", diff --git a/test/orange/orangeinp/CsgTreeUtils.test.cc b/test/orange/orangeinp/CsgTreeUtils.test.cc index c87f28407a..933846395f 100644 --- a/test/orange/orangeinp/CsgTreeUtils.test.cc +++ b/test/orange/orangeinp/CsgTreeUtils.test.cc @@ -7,7 +7,7 @@ #include "orange/orangeinp/CsgTreeUtils.hh" #include "orange/OrangeTypes.hh" -#include "orange/detail/LogicUtils.hh" +#include "orange/detail/LogicIO.hh" #include "orange/orangeinp/CsgTree.hh" #include "orange/orangeinp/CsgTypes.hh" #include "orange/orangeinp/detail/BuildLogic.hh" diff --git a/test/orange/univ/detail/InfixEvaluator.test.cc b/test/orange/univ/detail/InfixEvaluator.test.cc index deb52a2e9b..e2002103ba 100644 --- a/test/orange/univ/detail/InfixEvaluator.test.cc +++ b/test/orange/univ/detail/InfixEvaluator.test.cc @@ -6,10 +6,7 @@ //---------------------------------------------------------------------------// #include "orange/univ/detail/InfixEvaluator.hh" -#include - -#include "orange/detail/LogicUtils.hh" -#include "orange/univ/detail/LogicStack.hh" +#include "orange/detail/LogicIO.hh" #include "celeritas_test.hh" diff --git a/test/orange/univ/detail/LogicEvaluator.test.cc b/test/orange/univ/detail/LogicEvaluator.test.cc index 28335706f8..63477d41f8 100644 --- a/test/orange/univ/detail/LogicEvaluator.test.cc +++ b/test/orange/univ/detail/LogicEvaluator.test.cc @@ -6,8 +6,7 @@ //---------------------------------------------------------------------------// #include "orange/univ/detail/LogicEvaluator.hh" -#include "orange/detail/LogicUtils.hh" -#include "orange/univ/detail/LogicStack.hh" +#include "orange/detail/LogicIO.hh" #include "celeritas_test.hh" From 52d4d3380b0eab25ee66d3e7df9c211ba63f0ac3 Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Fri, 6 Feb 2026 07:05:04 -0500 Subject: [PATCH 15/24] Add single place for building "nowhere" logic expression --- src/orange/OrangeInputIO.json.cc | 2 +- src/orange/detail/ConvertLogic.hh | 2 +- src/orange/detail/LogicIO.cc | 19 +++++++++++++++++++ src/orange/detail/LogicIO.hh | 3 +++ src/orange/detail/UnitInserter.cc | 21 +++++++-------------- src/orange/orangeinp/UnitProto.cc | 10 ++++++---- 6 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/orange/OrangeInputIO.json.cc b/src/orange/OrangeInputIO.json.cc index 00eb13d095..798fee7a13 100644 --- a/src/orange/OrangeInputIO.json.cc +++ b/src/orange/OrangeInputIO.json.cc @@ -133,7 +133,7 @@ void from_json(nlohmann::json const& j, VolumeInput& value) { // Background volumes should be "nowhere" explicitly using "inside" // logic - value.logic = {logic::ltrue, logic::lnot}; + value.logic = detail::make_nowhere_expr(LogicNotation::postfix); value.bbox = {}; } else diff --git a/src/orange/detail/ConvertLogic.hh b/src/orange/detail/ConvertLogic.hh index c981816a5b..49562a8d8e 100644 --- a/src/orange/detail/ConvertLogic.hh +++ b/src/orange/detail/ConvertLogic.hh @@ -2,7 +2,7 @@ // Copyright Celeritas contributors: see top-level COPYRIGHT file for details // SPDX-License-Identifier: (Apache-2.0 OR MIT) //---------------------------------------------------------------------------// -//! \file orange/detail/LogicUtils.hh +//! \file orange/detail/ConvertLogic.hh //---------------------------------------------------------------------------// #pragma once diff --git a/src/orange/detail/LogicIO.cc b/src/orange/detail/LogicIO.cc index 4b0ed75cdd..68e56a57a3 100644 --- a/src/orange/detail/LogicIO.cc +++ b/src/orange/detail/LogicIO.cc @@ -120,6 +120,25 @@ std::vector string_to_logic(std::string_view s) return result; } +//---------------------------------------------------------------------------// +/*! + * Get a vector of logic indicating "nowhere". + */ +std::vector make_nowhere_expr(LogicNotation notation) +{ + CELER_EXPECT(notation != LogicNotation::size_); + + switch (notation) + { + case LogicNotation::postfix: + return {logic::ltrue, logic::lnot}; + case LogicNotation::infix: + return {logic::lnot, logic::ltrue}; + default: + CELER_ASSERT_UNREACHABLE(); + } +} + //---------------------------------------------------------------------------// } // namespace detail } // namespace celeritas diff --git a/src/orange/detail/LogicIO.hh b/src/orange/detail/LogicIO.hh index ccb2fc60ed..04ae5964ad 100644 --- a/src/orange/detail/LogicIO.hh +++ b/src/orange/detail/LogicIO.hh @@ -26,6 +26,9 @@ std::string logic_to_string(std::vector const& logic); // Build a logic definition from a string std::vector string_to_logic(std::string_view s); +// Get a vector of logic indicating "nowhere" +std::vector make_nowhere_expr(LogicNotation notation); + //---------------------------------------------------------------------------// } // namespace detail } // namespace celeritas diff --git a/src/orange/detail/UnitInserter.cc b/src/orange/detail/UnitInserter.cc index bed068989a..85e0a18cac 100644 --- a/src/orange/detail/UnitInserter.cc +++ b/src/orange/detail/UnitInserter.cc @@ -26,6 +26,7 @@ #include "corecel/sys/Environment.hh" #include "orange/OrangeData.hh" #include "orange/OrangeTypes.hh" +#include "orange/detail/LogicIO.hh" #include "UniverseInserter.hh" #include "../OrangeInput.hh" @@ -577,22 +578,14 @@ VolumeRecord UnitInserter::insert_volume(SurfacesRecord const& surf_record, } } - static auto const nowhere_logic = []() -> std::array { - if (orange_tracking_logic == LogicNotation::postfix) - return {logic::ltrue, logic::lnot}; - else - return {logic::lnot, logic::ltrue}; - }(); + static auto const nowhere_logic + = detail::make_nowhere_expr(orange_tracking_logic); - auto input_logic = make_span(v.logic); if (v.zorder == ZOrder::background) { // "Background" volumes should not be explicitly reachable by logic or // BIH - CELER_EXPECT(std::equal(input_logic.begin(), - input_logic.end(), - std::begin(nowhere_logic), - std::end(nowhere_logic))); + CELER_EXPECT(v.logic == nowhere_logic); CELER_EXPECT(!v.bbox); CELER_EXPECT(!v.obz); CELER_EXPECT(v.flags & VolumeRecord::implicit_vol); @@ -603,8 +596,7 @@ VolumeRecord UnitInserter::insert_volume(SurfacesRecord const& surf_record, VolumeRecord output; output.faces = local_surface_ids_.insert_back(v.faces.begin(), v.faces.end()); - output.logic - = logic_ints_.insert_back(input_logic.begin(), input_logic.end()); + output.logic = logic_ints_.insert_back(v.logic.begin(), v.logic.end()); output.max_intersections = static_cast(max_intersections); output.flags = v.flags; if (simple_safety) @@ -630,7 +622,8 @@ VolumeRecord UnitInserter::insert_volume(SurfacesRecord const& surf_record, } // Calculate the maximum stack depth of the volume definition - int depth = calc_depth(input_logic); + // TODO: is this valid for infix?? + int depth = calc_depth(make_span(v.logic)); CELER_VALIDATE(depth > 0, << "invalid logic definition: operators do not balance"); diff --git a/src/orange/orangeinp/UnitProto.cc b/src/orange/orangeinp/UnitProto.cc index 9430d585b4..8cba267508 100644 --- a/src/orange/orangeinp/UnitProto.cc +++ b/src/orange/orangeinp/UnitProto.cc @@ -25,6 +25,7 @@ #include "orange/OrangeData.hh" #include "orange/OrangeInput.hh" #include "orange/OrangeTypes.hh" +#include "orange/detail/LogicIO.hh" #include "orange/orangeinp/IntersectRegion.hh" #include "orange/transform/VariantTransform.hh" @@ -324,10 +325,11 @@ void UnitProto::build(ProtoBuilder& pb) const result.volumes.reserve(unit_volumes.size() + static_cast(csg_unit.background)); - // Always use postfix logic for unit input, post-processing to - // convert to tracking notation + // Always use postfix logic for unit input: post-processing in OrangeParams + // will convert to tracking notation + constexpr auto lgc_notation = LogicNotation::postfix; detail::DynamicBuildLogicPolicy const policy{ - LogicNotation::postfix, csg_unit.tree, &sorted_local_surfaces}; + lgc_notation, csg_unit.tree, &sorted_local_surfaces}; // Construct logic and faces with remapped surfaces for (auto vol_idx : range(unit_volumes.size())) { @@ -367,7 +369,7 @@ void UnitProto::build(ProtoBuilder& pb) const VolumeInput vi; vi.faces.resize(sorted_local_surfaces.size()); std::iota(vi.faces.begin(), vi.faces.end(), LocalSurfaceId{0}); - vi.logic = {logic::ltrue, logic::lnot}; + vi.logic = celeritas::detail::make_nowhere_expr(lgc_notation); vi.bbox = {}; // XXX: input converter changes to infinite bbox vi.zorder = ZOrder::background; /*! \todo The nearest internal surface is probably *not* the safety From b086d7464ed6024e6fbe23981bd34d2c628ead77 Mon Sep 17 00:00:00 2001 From: Seth R Johnson Date: Fri, 6 Feb 2026 07:07:46 -0500 Subject: [PATCH 16/24] Remove now-unnecessary logic from scalars --- src/orange/OrangeData.hh | 3 --- src/orange/OrangeParams.cc | 8 +++++--- src/orange/OrangeParamsOutput.cc | 1 - test/orange/OrangeJson.test.cc | 12 ++++++------ 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/orange/OrangeData.hh b/src/orange/OrangeData.hh index fce16e1883..a477bb4e11 100644 --- a/src/orange/OrangeData.hh +++ b/src/orange/OrangeData.hh @@ -61,9 +61,6 @@ struct OrangeParamsScalars // Soft comparison and dynamic "bumping" values Tolerance<> tol; - // Logic expression notation - LogicNotation logic{}; - // Raw pointers to externally owned memory for debug output OrangeParams const* host_geo_params{nullptr}; VolumeParams const* host_volume_params{nullptr}; diff --git a/src/orange/OrangeParams.cc b/src/orange/OrangeParams.cc index 71479a1777..cc1449c16c 100644 --- a/src/orange/OrangeParams.cc +++ b/src/orange/OrangeParams.cc @@ -180,6 +180,10 @@ OrangeParams::OrangeParams(OrangeInput&& input, SPConstVolumes&& volumes) << (celeritas::device() ? " and copying to GPU" : ""); ScopedTimeLog scoped_time; + // First, preprocess the input logic expressions to match the tracker + detail::convert_logic(input, orange_tracking_logic); + CELER_ASSERT(input.logic == orange_tracking_logic); + // Save global bounding box bbox_ = [&input] { auto& global = input.universes[orange_global_univ.unchecked_get()]; @@ -191,14 +195,11 @@ OrangeParams::OrangeParams(OrangeInput&& input, SPConstVolumes&& volumes) // Create host data for construction, setting tolerances first HostVal host_data; host_data.scalars.tol = input.tol; - host_data.scalars.logic = input.logic; host_data.scalars.num_univ_levels = detail::DepthCalculator{input.universes}(); host_data.scalars.num_vol_levels = volumes_ ? volumes_->num_volume_levels() : 0; - detail::convert_logic(input, orange_tracking_logic); - // Insert all universes { std::vector