Skip to content

Feat/analysis tools#273

Open
ggalgoczi wants to merge 17 commits into
mainfrom
feat/analysis-tools
Open

Feat/analysis tools#273
ggalgoczi wants to merge 17 commits into
mainfrom
feat/analysis-tools

Conversation

@ggalgoczi
Copy link
Copy Markdown
Contributor

Add GPU vs G4 comparison and visualization tools for optical photon validation. run_and_compare.py generates 7 diagnostic plots (hits, wavelength, time, positions, steps) with sqrt(N) error bars. plot_photon_paths.py provides 3D photon path visualization colored by wavelength. benchmark_apex.sh measures GPU vs G4 speedup and generates comparison plots in one command. Adds savephotonhistory config flag to control GPU/G4 hit .npy output.

Plots GPU photon paths from record.npy colored by wavelength.
Supports custom photon selection, sphere overlays, and wavelength
colorbar. Useful for visualizing WLS conversion and Rayleigh
scattering in optical simulations.
Runs GPURaytrace, collects GPU and G4 hits, and generates 6 comparison
plots: hit count, shifted wavelength, full wavelength, bulk arrival
time, full arrival time, and 3D hit positions. All with sqrt(N) error
bars. Can also run on pre-existing .npy files without re-simulating.
Shows number of optical boundary/surface steps each detected photon
takes before reaching the SiPM. GPU steps from record.npy, G4 steps
from extended hit array (when available). Uses sqrt(N) error bars.
Print a note when running simulations that DebugLite mode is needed
in the config JSON for step count plots, while HitPhoton is sufficient
for hit-only analysis.
Side-by-side comparison of opticks GPU and standalone G4 simulation hits.
Prints stats table (wavelength, time, position), statistical significance
test, and optional wavelength/time distribution histograms (--histograms).
LAr scintillator tile with two-stage WLS readout (pTP + blue WLS acrylic),
30 SiPMs along edge, Vikuiti reflective foil wrapping. Includes EFFICIENCY
and SensDet tags for SiPM hit collection.
When savephotonhistory=true in config JSON, saves full SEvt arrays
(photon, record, seq, hit) plus gpu_hits.npy and g4_hits.npy for
distribution comparison. G4 hits collected via thread-safe accumulator
in EndOfEventAction. GPURaytrace now accepts -c config flag.
@ggalgoczi ggalgoczi requested a review from plexoos April 4, 2026 02:56
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Cpp-linter Review

Used clang-format v20.1.2

Click here for the full clang-format patch
diff --git a/src/GPURaytrace.cpp b/src/GPURaytrace.cpp
index ddc3f49..9126d17 100644
--- a/src/GPURaytrace.cpp
+++ b/src/GPURaytrace.cpp
@@ -72,4 +72,4 @@ int main(int argc, char **argv)
-    program.add_argument("-c", "--config")
-        .help("config file name (without .json extension)")
-        .default_value(string(""))
-        .nargs(1);
+    program.add_argument( "-c", "--config" )
+        .help( "config file name (without .json extension)" )
+        .default_value( string( "" ) )
+        .nargs( 1 );
@@ -118,2 +118,2 @@ int main(int argc, char **argv)
-    string config_name = program.get<string>("--config");
-    if (!config_name.empty())
+    string config_name = program.get<string>( "--config" );
+    if( !config_name.empty() )
@@ -121 +121 @@ int main(int argc, char **argv)
-        gphox::Config cfg(config_name);
+        gphox::Config cfg( config_name );
diff --git a/src/GPURaytrace.h b/src/GPURaytrace.h
index 958777d..906740f 100644
--- a/src/GPURaytrace.h
+++ b/src/GPURaytrace.h
@@ -51 +51 @@ G4Mutex genstep_mutex = G4MUTEX_INITIALIZER;
-G4Mutex g4hits_mutex = G4MUTEX_INITIALIZER;
+G4Mutex                            g4hits_mutex  = G4MUTEX_INITIALIZER;
@@ -309 +309 @@ struct PrimaryGenerator : G4VUserPrimaryGeneratorAction
-        particle->SetKineticEnergy(10 * MeV);
+        particle->SetKineticEnergy( 10 * MeV );
@@ -337 +337,2 @@ struct EventAction : G4UserEventAction
-                if (!hc) continue;
+                if( !hc )
+                    continue;
@@ -339,2 +340,2 @@ struct EventAction : G4UserEventAction
-                PhotonHitsCollection *phc = dynamic_cast<PhotonHitsCollection*>(hc);
-                if (phc)
+                PhotonHitsCollection *phc = dynamic_cast<PhotonHitsCollection *>( hc );
+                if( phc )
@@ -342,2 +343,2 @@ struct EventAction : G4UserEventAction
-                    G4AutoLock lock(&g4hits_mutex);
-                    for (size_t j = 0; j < phc->entries(); j++)
+                    G4AutoLock lock( &g4hits_mutex );
+                    for( size_t j = 0; j < phc->entries(); j++ )
@@ -345,8 +346,6 @@ struct EventAction : G4UserEventAction
-                        PhotonHit* hit = (*phc)[j];
-                        float wl = 1239.84198f / static_cast<float>(hit->fenergy);
-                        g4_accumulated_hits.push_back({
-                            float(hit->fposition.x()), float(hit->fposition.y()), float(hit->fposition.z()), float(hit->ftime),
-                            float(hit->fdirection.x()), float(hit->fdirection.y()), float(hit->fdirection.z()), 0.f,
-                            float(hit->fpolarization.x()), float(hit->fpolarization.y()), float(hit->fpolarization.z()), wl,
-                            0.f, 0.f, 0.f, 0.f
-                        });
+                        PhotonHit *hit = ( *phc )[j];
+                        float      wl  = 1239.84198f / static_cast<float>( hit->fenergy );
+                        g4_accumulated_hits.push_back( { float( hit->fposition.x() ), float( hit->fposition.y() ), float( hit->fposition.z() ), float( hit->ftime ),
+                                                         float( hit->fdirection.x() ), float( hit->fdirection.y() ), float( hit->fdirection.z() ), 0.f,
+                                                         float( hit->fpolarization.x() ), float( hit->fpolarization.y() ), float( hit->fpolarization.z() ), wl,
+                                                         0.f, 0.f, 0.f, 0.f } );
@@ -373 +372 @@ struct RunAction : G4UserRunAction
-    bool fSavePhotonHistory{false};
+    bool         fSavePhotonHistory{ false };
@@ -406 +405 @@ struct RunAction : G4UserRunAction
-            if (fSavePhotonHistory)
+            if( fSavePhotonHistory )
@@ -414,2 +413,2 @@ struct RunAction : G4UserRunAction
-                    NP* gpu_h = NP::Make<float>(num_hits, 4, 4);
-                    for (unsigned idx = 0; idx < num_hits; idx++)
+                    NP *gpu_h = NP::Make<float>( num_hits, 4, 4 );
+                    for( unsigned idx = 0; idx < num_hits; idx++ )
@@ -418,2 +417,2 @@ struct RunAction : G4UserRunAction
-                        sev->getHit(hit, idx);
-                        memcpy(gpu_h->bytes() + idx * sizeof(sphoton), &hit, sizeof(sphoton));
+                        sev->getHit( hit, idx );
+                        memcpy( gpu_h->bytes() + idx * sizeof( sphoton ), &hit, sizeof( sphoton ) );
@@ -421 +420 @@ struct RunAction : G4UserRunAction
-                    gpu_h->save("gpu_hits.npy");
+                    gpu_h->save( "gpu_hits.npy" );
@@ -427,3 +426,3 @@ struct RunAction : G4UserRunAction
-                    G4AutoLock lock(&g4hits_mutex);
-                    size_t ng4 = g4_accumulated_hits.size();
-                    if (ng4 > 0)
+                    G4AutoLock lock( &g4hits_mutex );
+                    size_t     ng4 = g4_accumulated_hits.size();
+                    if( ng4 > 0 )
@@ -431,3 +430,3 @@ struct RunAction : G4UserRunAction
-                        NP* g4h = NP::Make<float>(ng4, 4, 4);
-                        memcpy(g4h->bytes(), g4_accumulated_hits.data(), ng4 * 16 * sizeof(float));
-                        g4h->save("g4_hits.npy");
+                        NP *g4h = NP::Make<float>( ng4, 4, 4 );
+                        memcpy( g4h->bytes(), g4_accumulated_hits.data(), ng4 * 16 * sizeof( float ) );
+                        g4h->save( "g4_hits.npy" );
diff --git a/src/config.cpp b/src/config.cpp
index 26cb221..312268d 100644
--- a/src/config.cpp
+++ b/src/config.cpp
@@ -140,2 +140,2 @@ void Config::ReadConfig(std::string filepath)
-    if (event_.contains("savephotonhistory"))
-      savephotonhistory = event_["savephotonhistory"].get<bool>();
+    if( event_.contains( "savephotonhistory" ) )
+        savephotonhistory = event_["savephotonhistory"].get<bool>();
diff --git a/src/config.h b/src/config.h
index 3dc2c1e..cbebea7 100644
--- a/src/config.h
+++ b/src/config.h
@@ -29 +29 @@ class Config
-  bool savephotonhistory{false};
+  bool savephotonhistory{ false };
@@ -31 +31 @@ class Config
- private:
+private:

Have any feedback or feature suggestions? Share it here.

Comment thread src/GPURaytrace.cpp
Comment on lines +72 to +75
program.add_argument("-c", "--config")
.help("config file name (without .json extension)")
.default_value(string(""))
.nargs(1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
program.add_argument("-c", "--config")
.help("config file name (without .json extension)")
.default_value(string(""))
.nargs(1);
program.add_argument( "-c", "--config" )
.help( "config file name (without .json extension)" )
.default_value( string( "" ) )
.nargs( 1 );

Comment thread src/GPURaytrace.cpp
Comment on lines +118 to +119
string config_name = program.get<string>("--config");
if (!config_name.empty())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
string config_name = program.get<string>("--config");
if (!config_name.empty())
string config_name = program.get<string>( "--config" );
if( !config_name.empty() )

Comment thread src/GPURaytrace.cpp
string config_name = program.get<string>("--config");
if (!config_name.empty())
{
gphox::Config cfg(config_name);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
gphox::Config cfg(config_name);
gphox::Config cfg( config_name );

Comment thread src/GPURaytrace.h
namespace
{
G4Mutex genstep_mutex = G4MUTEX_INITIALIZER;
G4Mutex g4hits_mutex = G4MUTEX_INITIALIZER;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
G4Mutex g4hits_mutex = G4MUTEX_INITIALIZER;
G4Mutex g4hits_mutex = G4MUTEX_INITIALIZER;

Comment thread src/GPURaytrace.h Outdated
Comment thread src/GPURaytrace.h
Comment on lines +427 to +429
G4AutoLock lock(&g4hits_mutex);
size_t ng4 = g4_accumulated_hits.size();
if (ng4 > 0)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
G4AutoLock lock(&g4hits_mutex);
size_t ng4 = g4_accumulated_hits.size();
if (ng4 > 0)
G4AutoLock lock( &g4hits_mutex );
size_t ng4 = g4_accumulated_hits.size();
if( ng4 > 0 )

Comment thread src/GPURaytrace.h Outdated
Comment thread src/config.cpp Outdated
Comment on lines +140 to +141
if (event_.contains("savephotonhistory"))
savephotonhistory = event_["savephotonhistory"].get<bool>();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
if (event_.contains("savephotonhistory"))
savephotonhistory = event_["savephotonhistory"].get<bool>();
if( event_.contains( "savephotonhistory" ) )
savephotonhistory = event_["savephotonhistory"].get<bool>();

Comment thread src/config.h Outdated
Comment thread src/config.h Outdated
@ggalgoczi ggalgoczi mentioned this pull request Apr 4, 2026
Plot both datasets at true bin centers instead of shifting ±17.5% of
bin width for readability.
@github-actions github-actions Bot dismissed their stale review April 7, 2026 14:39

outdated suggestion

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Cpp-linter Review

Used clang-format v20.1.2

Click here for the full clang-format patch
diff --git a/src/GPURaytrace.cpp b/src/GPURaytrace.cpp
index ddc3f49..9126d17 100644
--- a/src/GPURaytrace.cpp
+++ b/src/GPURaytrace.cpp
@@ -72,4 +72,4 @@ int main(int argc, char **argv)
-    program.add_argument("-c", "--config")
-        .help("config file name (without .json extension)")
-        .default_value(string(""))
-        .nargs(1);
+    program.add_argument( "-c", "--config" )
+        .help( "config file name (without .json extension)" )
+        .default_value( string( "" ) )
+        .nargs( 1 );
@@ -118,2 +118,2 @@ int main(int argc, char **argv)
-    string config_name = program.get<string>("--config");
-    if (!config_name.empty())
+    string config_name = program.get<string>( "--config" );
+    if( !config_name.empty() )
@@ -121 +121 @@ int main(int argc, char **argv)
-        gphox::Config cfg(config_name);
+        gphox::Config cfg( config_name );
diff --git a/src/GPURaytrace.h b/src/GPURaytrace.h
index 958777d..906740f 100644
--- a/src/GPURaytrace.h
+++ b/src/GPURaytrace.h
@@ -51 +51 @@ G4Mutex genstep_mutex = G4MUTEX_INITIALIZER;
-G4Mutex g4hits_mutex = G4MUTEX_INITIALIZER;
+G4Mutex                            g4hits_mutex  = G4MUTEX_INITIALIZER;
@@ -309 +309 @@ struct PrimaryGenerator : G4VUserPrimaryGeneratorAction
-        particle->SetKineticEnergy(10 * MeV);
+        particle->SetKineticEnergy( 10 * MeV );
@@ -337 +337,2 @@ struct EventAction : G4UserEventAction
-                if (!hc) continue;
+                if( !hc )
+                    continue;
@@ -339,2 +340,2 @@ struct EventAction : G4UserEventAction
-                PhotonHitsCollection *phc = dynamic_cast<PhotonHitsCollection*>(hc);
-                if (phc)
+                PhotonHitsCollection *phc = dynamic_cast<PhotonHitsCollection *>( hc );
+                if( phc )
@@ -342,2 +343,2 @@ struct EventAction : G4UserEventAction
-                    G4AutoLock lock(&g4hits_mutex);
-                    for (size_t j = 0; j < phc->entries(); j++)
+                    G4AutoLock lock( &g4hits_mutex );
+                    for( size_t j = 0; j < phc->entries(); j++ )
@@ -345,8 +346,6 @@ struct EventAction : G4UserEventAction
-                        PhotonHit* hit = (*phc)[j];
-                        float wl = 1239.84198f / static_cast<float>(hit->fenergy);
-                        g4_accumulated_hits.push_back({
-                            float(hit->fposition.x()), float(hit->fposition.y()), float(hit->fposition.z()), float(hit->ftime),
-                            float(hit->fdirection.x()), float(hit->fdirection.y()), float(hit->fdirection.z()), 0.f,
-                            float(hit->fpolarization.x()), float(hit->fpolarization.y()), float(hit->fpolarization.z()), wl,
-                            0.f, 0.f, 0.f, 0.f
-                        });
+                        PhotonHit *hit = ( *phc )[j];
+                        float      wl  = 1239.84198f / static_cast<float>( hit->fenergy );
+                        g4_accumulated_hits.push_back( { float( hit->fposition.x() ), float( hit->fposition.y() ), float( hit->fposition.z() ), float( hit->ftime ),
+                                                         float( hit->fdirection.x() ), float( hit->fdirection.y() ), float( hit->fdirection.z() ), 0.f,
+                                                         float( hit->fpolarization.x() ), float( hit->fpolarization.y() ), float( hit->fpolarization.z() ), wl,
+                                                         0.f, 0.f, 0.f, 0.f } );
@@ -373 +372 @@ struct RunAction : G4UserRunAction
-    bool fSavePhotonHistory{false};
+    bool         fSavePhotonHistory{ false };
@@ -406 +405 @@ struct RunAction : G4UserRunAction
-            if (fSavePhotonHistory)
+            if( fSavePhotonHistory )
@@ -414,2 +413,2 @@ struct RunAction : G4UserRunAction
-                    NP* gpu_h = NP::Make<float>(num_hits, 4, 4);
-                    for (unsigned idx = 0; idx < num_hits; idx++)
+                    NP *gpu_h = NP::Make<float>( num_hits, 4, 4 );
+                    for( unsigned idx = 0; idx < num_hits; idx++ )
@@ -418,2 +417,2 @@ struct RunAction : G4UserRunAction
-                        sev->getHit(hit, idx);
-                        memcpy(gpu_h->bytes() + idx * sizeof(sphoton), &hit, sizeof(sphoton));
+                        sev->getHit( hit, idx );
+                        memcpy( gpu_h->bytes() + idx * sizeof( sphoton ), &hit, sizeof( sphoton ) );
@@ -421 +420 @@ struct RunAction : G4UserRunAction
-                    gpu_h->save("gpu_hits.npy");
+                    gpu_h->save( "gpu_hits.npy" );
@@ -427,3 +426,3 @@ struct RunAction : G4UserRunAction
-                    G4AutoLock lock(&g4hits_mutex);
-                    size_t ng4 = g4_accumulated_hits.size();
-                    if (ng4 > 0)
+                    G4AutoLock lock( &g4hits_mutex );
+                    size_t     ng4 = g4_accumulated_hits.size();
+                    if( ng4 > 0 )
@@ -431,3 +430,3 @@ struct RunAction : G4UserRunAction
-                        NP* g4h = NP::Make<float>(ng4, 4, 4);
-                        memcpy(g4h->bytes(), g4_accumulated_hits.data(), ng4 * 16 * sizeof(float));
-                        g4h->save("g4_hits.npy");
+                        NP *g4h = NP::Make<float>( ng4, 4, 4 );
+                        memcpy( g4h->bytes(), g4_accumulated_hits.data(), ng4 * 16 * sizeof( float ) );
+                        g4h->save( "g4_hits.npy" );
diff --git a/src/config.cpp b/src/config.cpp
index 26cb221..312268d 100644
--- a/src/config.cpp
+++ b/src/config.cpp
@@ -140,2 +140,2 @@ void Config::ReadConfig(std::string filepath)
-    if (event_.contains("savephotonhistory"))
-      savephotonhistory = event_["savephotonhistory"].get<bool>();
+    if( event_.contains( "savephotonhistory" ) )
+        savephotonhistory = event_["savephotonhistory"].get<bool>();
diff --git a/src/config.h b/src/config.h
index 3dc2c1e..cbebea7 100644
--- a/src/config.h
+++ b/src/config.h
@@ -29 +29 @@ class Config
-  bool savephotonhistory{false};
+  bool savephotonhistory{ false };
@@ -31 +31 @@ class Config
- private:
+private:

Have any feedback or feature suggestions? Share it here.

Comment thread src/GPURaytrace.cpp
Comment on lines +72 to +75
program.add_argument("-c", "--config")
.help("config file name (without .json extension)")
.default_value(string(""))
.nargs(1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
program.add_argument("-c", "--config")
.help("config file name (without .json extension)")
.default_value(string(""))
.nargs(1);
program.add_argument( "-c", "--config" )
.help( "config file name (without .json extension)" )
.default_value( string( "" ) )
.nargs( 1 );

Comment thread src/GPURaytrace.cpp
Comment on lines +118 to +119
string config_name = program.get<string>("--config");
if (!config_name.empty())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
string config_name = program.get<string>("--config");
if (!config_name.empty())
string config_name = program.get<string>( "--config" );
if( !config_name.empty() )

Comment thread src/GPURaytrace.cpp
string config_name = program.get<string>("--config");
if (!config_name.empty())
{
gphox::Config cfg(config_name);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
gphox::Config cfg(config_name);
gphox::Config cfg( config_name );

Comment thread src/GPURaytrace.h
namespace
{
G4Mutex genstep_mutex = G4MUTEX_INITIALIZER;
G4Mutex g4hits_mutex = G4MUTEX_INITIALIZER;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
G4Mutex g4hits_mutex = G4MUTEX_INITIALIZER;
G4Mutex g4hits_mutex = G4MUTEX_INITIALIZER;

Comment thread src/GPURaytrace.h Outdated
Comment thread src/GPURaytrace.h
Comment on lines +427 to +429
G4AutoLock lock(&g4hits_mutex);
size_t ng4 = g4_accumulated_hits.size();
if (ng4 > 0)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
G4AutoLock lock(&g4hits_mutex);
size_t ng4 = g4_accumulated_hits.size();
if (ng4 > 0)
G4AutoLock lock( &g4hits_mutex );
size_t ng4 = g4_accumulated_hits.size();
if( ng4 > 0 )

Comment thread src/GPURaytrace.h Outdated
Comment thread src/config.cpp Outdated
Comment thread src/config.h Outdated
Comment thread src/config.h Outdated
The 10 MeV change was for APEX validation and belongs on the
physics branch, not analysis-tools.
@github-actions github-actions Bot dismissed their stale review April 7, 2026 14:56

outdated suggestion

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Cpp-linter Review

Used clang-format v20.1.2

Click here for the full clang-format patch
diff --git a/src/GPURaytrace.cpp b/src/GPURaytrace.cpp
index ddc3f49..9126d17 100644
--- a/src/GPURaytrace.cpp
+++ b/src/GPURaytrace.cpp
@@ -72,4 +72,4 @@ int main(int argc, char **argv)
-    program.add_argument("-c", "--config")
-        .help("config file name (without .json extension)")
-        .default_value(string(""))
-        .nargs(1);
+    program.add_argument( "-c", "--config" )
+        .help( "config file name (without .json extension)" )
+        .default_value( string( "" ) )
+        .nargs( 1 );
@@ -118,2 +118,2 @@ int main(int argc, char **argv)
-    string config_name = program.get<string>("--config");
-    if (!config_name.empty())
+    string config_name = program.get<string>( "--config" );
+    if( !config_name.empty() )
@@ -121 +121 @@ int main(int argc, char **argv)
-        gphox::Config cfg(config_name);
+        gphox::Config cfg( config_name );
diff --git a/src/GPURaytrace.h b/src/GPURaytrace.h
index c01f8a2..8f1ba31 100644
--- a/src/GPURaytrace.h
+++ b/src/GPURaytrace.h
@@ -51 +51 @@ G4Mutex genstep_mutex = G4MUTEX_INITIALIZER;
-G4Mutex g4hits_mutex = G4MUTEX_INITIALIZER;
+G4Mutex                            g4hits_mutex  = G4MUTEX_INITIALIZER;
@@ -337 +337,2 @@ struct EventAction : G4UserEventAction
-                if (!hc) continue;
+                if( !hc )
+                    continue;
@@ -339,2 +340,2 @@ struct EventAction : G4UserEventAction
-                PhotonHitsCollection *phc = dynamic_cast<PhotonHitsCollection*>(hc);
-                if (phc)
+                PhotonHitsCollection *phc = dynamic_cast<PhotonHitsCollection *>( hc );
+                if( phc )
@@ -342,2 +343,2 @@ struct EventAction : G4UserEventAction
-                    G4AutoLock lock(&g4hits_mutex);
-                    for (size_t j = 0; j < phc->entries(); j++)
+                    G4AutoLock lock( &g4hits_mutex );
+                    for( size_t j = 0; j < phc->entries(); j++ )
@@ -345,8 +346,6 @@ struct EventAction : G4UserEventAction
-                        PhotonHit* hit = (*phc)[j];
-                        float wl = 1239.84198f / static_cast<float>(hit->fenergy);
-                        g4_accumulated_hits.push_back({
-                            float(hit->fposition.x()), float(hit->fposition.y()), float(hit->fposition.z()), float(hit->ftime),
-                            float(hit->fdirection.x()), float(hit->fdirection.y()), float(hit->fdirection.z()), 0.f,
-                            float(hit->fpolarization.x()), float(hit->fpolarization.y()), float(hit->fpolarization.z()), wl,
-                            0.f, 0.f, 0.f, 0.f
-                        });
+                        PhotonHit *hit = ( *phc )[j];
+                        float      wl  = 1239.84198f / static_cast<float>( hit->fenergy );
+                        g4_accumulated_hits.push_back( { float( hit->fposition.x() ), float( hit->fposition.y() ), float( hit->fposition.z() ), float( hit->ftime ),
+                                                         float( hit->fdirection.x() ), float( hit->fdirection.y() ), float( hit->fdirection.z() ), 0.f,
+                                                         float( hit->fpolarization.x() ), float( hit->fpolarization.y() ), float( hit->fpolarization.z() ), wl,
+                                                         0.f, 0.f, 0.f, 0.f } );
@@ -373 +372 @@ struct RunAction : G4UserRunAction
-    bool fSavePhotonHistory{false};
+    bool         fSavePhotonHistory{ false };
@@ -406 +405 @@ struct RunAction : G4UserRunAction
-            if (fSavePhotonHistory)
+            if( fSavePhotonHistory )
@@ -414,2 +413,2 @@ struct RunAction : G4UserRunAction
-                    NP* gpu_h = NP::Make<float>(num_hits, 4, 4);
-                    for (unsigned idx = 0; idx < num_hits; idx++)
+                    NP *gpu_h = NP::Make<float>( num_hits, 4, 4 );
+                    for( unsigned idx = 0; idx < num_hits; idx++ )
@@ -418,2 +417,2 @@ struct RunAction : G4UserRunAction
-                        sev->getHit(hit, idx);
-                        memcpy(gpu_h->bytes() + idx * sizeof(sphoton), &hit, sizeof(sphoton));
+                        sev->getHit( hit, idx );
+                        memcpy( gpu_h->bytes() + idx * sizeof( sphoton ), &hit, sizeof( sphoton ) );
@@ -421 +420 @@ struct RunAction : G4UserRunAction
-                    gpu_h->save("gpu_hits.npy");
+                    gpu_h->save( "gpu_hits.npy" );
@@ -427,3 +426,3 @@ struct RunAction : G4UserRunAction
-                    G4AutoLock lock(&g4hits_mutex);
-                    size_t ng4 = g4_accumulated_hits.size();
-                    if (ng4 > 0)
+                    G4AutoLock lock( &g4hits_mutex );
+                    size_t     ng4 = g4_accumulated_hits.size();
+                    if( ng4 > 0 )
@@ -431,3 +430,3 @@ struct RunAction : G4UserRunAction
-                        NP* g4h = NP::Make<float>(ng4, 4, 4);
-                        memcpy(g4h->bytes(), g4_accumulated_hits.data(), ng4 * 16 * sizeof(float));
-                        g4h->save("g4_hits.npy");
+                        NP *g4h = NP::Make<float>( ng4, 4, 4 );
+                        memcpy( g4h->bytes(), g4_accumulated_hits.data(), ng4 * 16 * sizeof( float ) );
+                        g4h->save( "g4_hits.npy" );
diff --git a/src/config.cpp b/src/config.cpp
index 26cb221..312268d 100644
--- a/src/config.cpp
+++ b/src/config.cpp
@@ -140,2 +140,2 @@ void Config::ReadConfig(std::string filepath)
-    if (event_.contains("savephotonhistory"))
-      savephotonhistory = event_["savephotonhistory"].get<bool>();
+    if( event_.contains( "savephotonhistory" ) )
+        savephotonhistory = event_["savephotonhistory"].get<bool>();
diff --git a/src/config.h b/src/config.h
index 3dc2c1e..cbebea7 100644
--- a/src/config.h
+++ b/src/config.h
@@ -29 +29 @@ class Config
-  bool savephotonhistory{false};
+  bool savephotonhistory{ false };
@@ -31 +31 @@ class Config
- private:
+private:

Have any feedback or feature suggestions? Share it here.

Comment thread src/GPURaytrace.cpp
Comment on lines +72 to +75
program.add_argument("-c", "--config")
.help("config file name (without .json extension)")
.default_value(string(""))
.nargs(1);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
program.add_argument("-c", "--config")
.help("config file name (without .json extension)")
.default_value(string(""))
.nargs(1);
program.add_argument( "-c", "--config" )
.help( "config file name (without .json extension)" )
.default_value( string( "" ) )
.nargs( 1 );

Comment thread src/GPURaytrace.cpp
Comment on lines +118 to +119
string config_name = program.get<string>("--config");
if (!config_name.empty())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
string config_name = program.get<string>("--config");
if (!config_name.empty())
string config_name = program.get<string>( "--config" );
if( !config_name.empty() )

Comment thread src/GPURaytrace.cpp
string config_name = program.get<string>("--config");
if (!config_name.empty())
{
gphox::Config cfg(config_name);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
gphox::Config cfg(config_name);
gphox::Config cfg( config_name );

Comment thread src/GPURaytrace.h
namespace
{
G4Mutex genstep_mutex = G4MUTEX_INITIALIZER;
G4Mutex g4hits_mutex = G4MUTEX_INITIALIZER;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
G4Mutex g4hits_mutex = G4MUTEX_INITIALIZER;
G4Mutex g4hits_mutex = G4MUTEX_INITIALIZER;

Comment thread src/GPURaytrace.h Outdated
{
G4VHitsCollection *hc = hce->GetHC(i);
if (hc)
if (!hc) continue;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
if (!hc) continue;
if( !hc )
continue;

Comment thread src/GPURaytrace.h
Comment on lines +427 to +429
G4AutoLock lock(&g4hits_mutex);
size_t ng4 = g4_accumulated_hits.size();
if (ng4 > 0)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
G4AutoLock lock(&g4hits_mutex);
size_t ng4 = g4_accumulated_hits.size();
if (ng4 > 0)
G4AutoLock lock( &g4hits_mutex );
size_t ng4 = g4_accumulated_hits.size();
if( ng4 > 0 )

Comment thread src/GPURaytrace.h Outdated
Comment thread src/config.cpp Outdated
Comment on lines +140 to +141
if (event_.contains("savephotonhistory"))
savephotonhistory = event_["savephotonhistory"].get<bool>();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
if (event_.contains("savephotonhistory"))
savephotonhistory = event_["savephotonhistory"].get<bool>();
if( event_.contains( "savephotonhistory" ) )
savephotonhistory = event_["savephotonhistory"].get<bool>();

Comment thread src/config.h Outdated

storch torch;

bool savephotonhistory{false};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
bool savephotonhistory{false};
bool savephotonhistory{ false };

Comment thread src/config.h Outdated

bool savephotonhistory{false};

private:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
private:
private:

APEX liquid argon scintillator tile with WLS light guide, Vikuiti
reflective foils, and 31 SiPMs. Used for GPU vs G4 benchmarking.
@github-actions github-actions Bot dismissed their stale review April 9, 2026 22:56

outdated suggestion

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Cpp-linter Review

Used clang-format v20.1.2

Click here for the full clang-format patch
diff --git a/src/GPURaytrace.cpp b/src/GPURaytrace.cpp
index ddc3f49..f0590db 100644
--- a/src/GPURaytrace.cpp
+++ b/src/GPURaytrace.cpp
@@ -15 +14,0 @@
-#include "config.h"
@@ -16,0 +16 @@
+#include "config.h"
diff --git a/src/GPURaytrace.h b/src/GPURaytrace.h
index c01f8a2..9619191 100644
--- a/src/GPURaytrace.h
+++ b/src/GPURaytrace.h
@@ -337 +337,2 @@ struct EventAction : G4UserEventAction
-                if (!hc) continue;
+                if (!hc)
+                    continue;
@@ -339 +340 @@ struct EventAction : G4UserEventAction
-                PhotonHitsCollection *phc = dynamic_cast<PhotonHitsCollection*>(hc);
+                PhotonHitsCollection *phc = dynamic_cast<PhotonHitsCollection *>(hc);
@@ -345 +346 @@ struct EventAction : G4UserEventAction
-                        PhotonHit* hit = (*phc)[j];
+                        PhotonHit *hit = (*phc)[j];
@@ -347,6 +348,5 @@ struct EventAction : G4UserEventAction
-                        g4_accumulated_hits.push_back({
-                            float(hit->fposition.x()), float(hit->fposition.y()), float(hit->fposition.z()), float(hit->ftime),
-                            float(hit->fdirection.x()), float(hit->fdirection.y()), float(hit->fdirection.z()), 0.f,
-                            float(hit->fpolarization.x()), float(hit->fpolarization.y()), float(hit->fpolarization.z()), wl,
-                            0.f, 0.f, 0.f, 0.f
-                        });
+                        g4_accumulated_hits.push_back(
+                            {float(hit->fposition.x()), float(hit->fposition.y()), float(hit->fposition.z()),
+                             float(hit->ftime), float(hit->fdirection.x()), float(hit->fdirection.y()),
+                             float(hit->fdirection.z()), 0.f, float(hit->fpolarization.x()),
+                             float(hit->fpolarization.y()), float(hit->fpolarization.z()), wl, 0.f, 0.f, 0.f, 0.f});
@@ -414 +414 @@ struct RunAction : G4UserRunAction
-                    NP* gpu_h = NP::Make<float>(num_hits, 4, 4);
+                    NP *gpu_h = NP::Make<float>(num_hits, 4, 4);
@@ -431 +431 @@ struct RunAction : G4UserRunAction
-                        NP* g4h = NP::Make<float>(ng4, 4, 4);
+                        NP *g4h = NP::Make<float>(ng4, 4, 4);
diff --git a/src/config.cpp b/src/config.cpp
index 26cb221..1fe8c36 100644
--- a/src/config.cpp
+++ b/src/config.cpp
@@ -141 +141 @@ void Config::ReadConfig(std::string filepath)
-      savephotonhistory = event_["savephotonhistory"].get<bool>();
+        savephotonhistory = event_["savephotonhistory"].get<bool>();
diff --git a/src/config.h b/src/config.h
index 3dc2c1e..fe92a2c 100644
--- a/src/config.h
+++ b/src/config.h
@@ -31 +31 @@ class Config
- private:
+private:

Have any feedback or feature suggestions? Share it here.

Comment thread src/GPURaytrace.cpp Outdated

#include "sysrap/OPTICKS_LOG.hh"

#include "config.h"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Please remove the line(s)

  • 15

Comment thread src/GPURaytrace.cpp
#include "sysrap/OPTICKS_LOG.hh"

#include "config.h"
#include "GPURaytrace.h"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
#include "GPURaytrace.h"
#include "config.h"

Comment thread src/GPURaytrace.h Outdated
{
G4VHitsCollection *hc = hce->GetHC(i);
if (hc)
if (!hc) continue;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
if (!hc) continue;
if (!hc)
continue;

Comment thread src/GPURaytrace.h Outdated
if (hc)
if (!hc) continue;

PhotonHitsCollection *phc = dynamic_cast<PhotonHitsCollection*>(hc);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
PhotonHitsCollection *phc = dynamic_cast<PhotonHitsCollection*>(hc);
PhotonHitsCollection *phc = dynamic_cast<PhotonHitsCollection *>(hc);

Comment thread src/GPURaytrace.h Outdated
G4AutoLock lock(&g4hits_mutex);
for (size_t j = 0; j < phc->entries(); j++)
{
PhotonHit* hit = (*phc)[j];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
PhotonHit* hit = (*phc)[j];
PhotonHit *hit = (*phc)[j];

Comment thread src/GPURaytrace.h Outdated
Comment on lines +347 to +352
g4_accumulated_hits.push_back({
float(hit->fposition.x()), float(hit->fposition.y()), float(hit->fposition.z()), float(hit->ftime),
float(hit->fdirection.x()), float(hit->fdirection.y()), float(hit->fdirection.z()), 0.f,
float(hit->fpolarization.x()), float(hit->fpolarization.y()), float(hit->fpolarization.z()), wl,
0.f, 0.f, 0.f, 0.f
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
g4_accumulated_hits.push_back({
float(hit->fposition.x()), float(hit->fposition.y()), float(hit->fposition.z()), float(hit->ftime),
float(hit->fdirection.x()), float(hit->fdirection.y()), float(hit->fdirection.z()), 0.f,
float(hit->fpolarization.x()), float(hit->fpolarization.y()), float(hit->fpolarization.z()), wl,
0.f, 0.f, 0.f, 0.f
});
g4_accumulated_hits.push_back(
{float(hit->fposition.x()), float(hit->fposition.y()), float(hit->fposition.z()),
float(hit->ftime), float(hit->fdirection.x()), float(hit->fdirection.y()),
float(hit->fdirection.z()), 0.f, float(hit->fpolarization.x()),
float(hit->fpolarization.y()), float(hit->fpolarization.z()), wl, 0.f, 0.f, 0.f, 0.f});

Comment thread src/GPURaytrace.h Outdated
// Save GPU hits as .npy (sphoton layout: N x 4 x 4 float32)
{
theCreationProcessid = 1;
NP* gpu_h = NP::Make<float>(num_hits, 4, 4);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
NP* gpu_h = NP::Make<float>(num_hits, 4, 4);
NP *gpu_h = NP::Make<float>(num_hits, 4, 4);

Comment thread src/GPURaytrace.h Outdated
size_t ng4 = g4_accumulated_hits.size();
if (ng4 > 0)
{
NP* g4h = NP::Make<float>(ng4, 4, 4);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
NP* g4h = NP::Make<float>(ng4, 4, 4);
NP *g4h = NP::Make<float>(ng4, 4, 4);

Comment thread src/config.cpp Outdated
SEventConfig::SetMaxSlot( event_["maxslot"] );

if (event_.contains("savephotonhistory"))
savephotonhistory = event_["savephotonhistory"].get<bool>();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
savephotonhistory = event_["savephotonhistory"].get<bool>();
savephotonhistory = event_["savephotonhistory"].get<bool>();

Comment thread src/config.h Outdated

bool savephotonhistory{false};

private:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-format suggestion

Suggested change
private:
private:

Comment thread benchmarks/benchmark_apex.sh
Comment thread src/config.h Outdated
@@ -26,6 +26,8 @@ class Config

storch torch;

bool savephotonhistory{false};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please comply to microsoft style convention. Function names should be CamelCase.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure. You asked me to separate formatting and coding PRs. Therefore this will be an other PR once this is merged, correct?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a new line of code, so it should be formatted according to the agreed coding style. The cpp-linter is set up to detect that. If it complains about existing code, you can safely ignore it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Got it, fixed now

Comment thread tests/geom/apex.gdml
@github-actions github-actions Bot dismissed their stale review April 12, 2026 23:08

outdated suggestion

…speedup.py

Move the inline python3 -c "..." block out of benchmarks/benchmark_apex.sh
into optiphy/ana/print_speedup.py. Shell now only runs the benchmark and
parses log values; Python module owns the formatting and is testable
standalone. Behavior verified identical against same input values.
@ggalgoczi
Copy link
Copy Markdown
Contributor Author

@plexoos one of the failing tests (ubuntu22.04, 12.1.1, 8.0.0, 11.3.2, 3.22.1) failed due to

154 - QUDARapTest.QEvt_Lifecycle_Test (Failed)

I think I did not touch it. Let me know if you saw this error before.

@plexoos
Copy link
Copy Markdown
Member

plexoos commented Apr 15, 2026

Please update or rebase the branch, and change the PR title to better reflect the intended change using conventional commit format.

@ggalgoczi
Copy link
Copy Markdown
Contributor Author

Please update or rebase the branch, and change the PR title to better reflect the intended change using conventional commit format.

I'll rebase 271-273 once 269 and 270 are merged — otherwise they'll just need rebasing again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants