Safeguard for Opticks binary tree size

[ggalgoczi]

Stop Opticks geometry read-in if binary tree size blows up. Warn user and suggest solution.

Solves #77

Test:
./build/src/simg4ox -g dune_forward_geometry_2025_april_replaced_elliptical_tubes.gdml -m esi-g4ox/run.mac

Exits with error as expected:

simg4ox: /esi/eic-opticks/CSG/CSGImport.cc:312: CSGPrim* CSGImport::importPrim(int, const snode&): Assertion `bn < 100000' failed.

Fixed DUNE geometry works fine, no exit:
./build/src/simg4ox -g duneforwarddetector3.gdml -m esi-g4ox/run.mac

[github-actions]

Cpp-linter Review

Used clang-format v16.0.6

Click here for the full clang-format patch

diff --git a/CSG/CSGImport.cc b/CSG/CSGImport.cc
index bad78b6..eeeed9a 100644
--- a/CSG/CSGImport.cc
+++ b/CSG/CSGImport.cc
@@ -311 +311 @@ CSGPrim* CSGImport::importPrim(int primIdx, const snode& node )
-    
+
@@ -313,2 +313,5 @@ CSGPrim* CSGImport::importPrim(int primIdx, const snode& node )
-        {
-        std::cerr << "Too many nodes in binary tree (more than a 100 000) built by Opticks. Likely a nested subtraction/union solid that Opticks turns into a full binary tree. Check your geometry as described in: https://github.com/BNLNPPS/esi-g4ox/issues/127" << std::endl;
+    {
+        std::cerr << "Too many nodes in binary tree (more than a 100 000) built by Opticks. Likely a nested "
+                     "subtraction/union solid that Opticks turns into a full binary tree. Check your geometry as "
+                     "described in: https://github.com/BNLNPPS/esi-g4ox/issues/127"
+                  << std::endl;
@@ -316 +319 @@ CSGPrim* CSGImport::importPrim(int primIdx, const snode& node )
-        }
+    }

Have any feedback or feature suggestions? Share it here.

outdated suggestion

outdated suggestion

[github-actions]

Cpp-linter Review

Used clang-format v16.0.6

Click here for the full clang-format patch

diff --git a/CSG/CSGImport.cc b/CSG/CSGImport.cc
index 7015a7e..6c75da1 100644
--- a/CSG/CSGImport.cc
+++ b/CSG/CSGImport.cc
@@ -311,7 +311,7 @@ CSGPrim* CSGImport::importPrim(int primIdx, const snode& node )
-    
-    if (bn > 100000)                     // Check if binary tree blows up and warn user
-    LOG(WARNING) << "Too many nodes in binary tree (more than a 100 000) built by Opticks. Likely a nested "
-                     "subtraction/union solid that Opticks turns into a full binary tree. Check your geometry as "
-                     "described in: https://github.com/BNLNPPS/esi-g4ox/issues/127";   
-    
-    // 2. count total subs for any listnodes of this lvid 
+
+    if (bn > 100000) // Check if binary tree blows up and warn user
+        LOG(WARNING) << "Too many nodes in binary tree (more than a 100 000) built by Opticks. Likely a nested "
+                        "subtraction/union solid that Opticks turns into a full binary tree. Check your geometry as "
+                        "described in: https://github.com/BNLNPPS/esi-g4ox/issues/127";
+
+    // 2. count total subs for any listnodes of this lvid

Have any feedback or feature suggestions? Share it here.

outdated suggestion

[plexoos]

It’s actually lowercase ‘warning’. you shouldn’t have trusted me on that.

[ggalgoczi]

@plexoos is there anything else needed to merge this feature?

[plexoos]

It’s probably a stretch to call this a feature :) but if you’d like to include the warning with the hard-coded condition on the number of volumes, let’s go ahead and merge.

[github-actions]

Cpp-linter Review

Used clang-format v18.1.3

Click here for the full clang-format patch

diff --git a/CSG/CSGImport.cc b/CSG/CSGImport.cc
index aeca179..38acdde 100644
--- a/CSG/CSGImport.cc
+++ b/CSG/CSGImport.cc
@@ -320,3 +320,3 @@ CSGPrim* CSGImport::importPrim(int primIdx, const snode& node )
-    std::vector<const sn*> lns ; 
-    sn::GetLVListnodes( lns, lvid ); 
-    int num_sub_total = sn::GetChildTotal( lns ); 
+    std::vector<const sn *> lns;
+    sn::GetLVListnodes(lns, lvid);
+    int num_sub_total = sn::GetChildTotal(lns);

Have any feedback or feature suggestions? Share it here.

outdated suggestion

[ggalgoczi]

@plexoos please review the change (had to move one space due to clang)