style: fix clang-format violations in src/GPURaytrace.h by ggalgoczi · Pull Request #272 · BNLNPPS/eic-opticks

ggalgoczi · 2026-04-04T02:55:38Z

The cpp-linter CI job flagged two Microsoft style violations in src/GPURaytrace.h introduced by the scintillation genstep fix.

Line 527: Wrap tcKeys initializer list — kSCINTILLATIONTIMECONSTANT3 moved to continuation line
Lines 540–542: Collapse multi-line ternary into single-line form per Microsoft style

// Before
const G4int tcKeys[3] = {kSCINTILLATIONTIMECONSTANT1, kSCINTILLATIONTIMECONSTANT2, kSCINTILLATIONTIMECONSTANT3};
yield[c] = MPT->ConstPropertyExists(yieldKeys[c])
               ? MPT->GetConstProperty(yieldKeys[c])
               : (c == 0 ? 1.0 : 0.0);

// After
const G4int tcKeys[3] = {kSCINTILLATIONTIMECONSTANT1, kSCINTILLATIONTIMECONSTANT2,
                         kSCINTILLATIONTIMECONSTANT3};
yield[c] = MPT->ConstPropertyExists(yieldKeys[c]) ? MPT->GetConstProperty(yieldKeys[c])
                                                  : (c == 0 ? 1.0 : 0.0);

github-actions

Cpp-linter Review

Used clang-format v20.1.2

Click here for the full clang-format patch

diff --git a/src/GPURaytrace.h b/src/GPURaytrace.h
index 03318df..027866d 100644
--- a/src/GPURaytrace.h
+++ b/src/GPURaytrace.h
@@ -527,2 +527,2 @@ struct SteppingAction : G4UserSteppingAction
-                        const G4int tcKeys[3] = {kSCINTILLATIONTIMECONSTANT1, kSCINTILLATIONTIMECONSTANT2, kSCINTILLATIONTIMECONSTANT3};
-                        const G4int yieldKeys[3] = {kSCINTILLATIONYIELD1, kSCINTILLATIONYIELD2, kSCINTILLATIONYIELD3};
+                        const G4int tcKeys[3]    = { kSCINTILLATIONTIMECONSTANT1, kSCINTILLATIONTIMECONSTANT2, kSCINTILLATIONTIMECONSTANT3 };
+                        const G4int yieldKeys[3] = { kSCINTILLATIONYIELD1, kSCINTILLATIONYIELD2, kSCINTILLATIONYIELD3 };
@@ -530,2 +530,2 @@ struct SteppingAction : G4UserSteppingAction
-                        G4double tc[3] = {0, 0, 0};
-                        G4double yield[3] = {0, 0, 0};
+                        G4double tc[3]    = { 0, 0, 0 };
+                        G4double yield[3] = { 0, 0, 0 };
@@ -533 +533 @@ struct SteppingAction : G4UserSteppingAction
-                        G4int nComp = 0;
+                        G4int    nComp    = 0;
@@ -535 +535 @@ struct SteppingAction : G4UserSteppingAction
-                        for (G4int c = 0; c < 3; c++)
+                        for( G4int c = 0; c < 3; c++ )
@@ -537 +537 @@ struct SteppingAction : G4UserSteppingAction
-                            if (MPT->ConstPropertyExists(tcKeys[c]))
+                            if( MPT->ConstPropertyExists( tcKeys[c] ) )
@@ -539,4 +539,4 @@ struct SteppingAction : G4UserSteppingAction
-                                tc[c] = MPT->GetConstProperty(tcKeys[c]);
-                                yield[c] = MPT->ConstPropertyExists(yieldKeys[c])
-                                               ? MPT->GetConstProperty(yieldKeys[c])
-                                               : (c == 0 ? 1.0 : 0.0);
+                                tc[c]    = MPT->GetConstProperty( tcKeys[c] );
+                                yield[c] = MPT->ConstPropertyExists( yieldKeys[c] )
+                                               ? MPT->GetConstProperty( yieldKeys[c] )
+                                               : ( c == 0 ? 1.0 : 0.0 );
@@ -548,3 +548,3 @@ struct SteppingAction : G4UserSteppingAction
-                        G4AutoLock lock(&genstep_mutex);
-                        G4int nRemaining = fNumPhotons;
-                        for (G4int c = 0; c < nComp; c++)
+                        G4AutoLock lock( &genstep_mutex );
+                        G4int      nRemaining = fNumPhotons;
+                        for( G4int c = 0; c < nComp; c++ )
@@ -553 +553 @@ struct SteppingAction : G4UserSteppingAction
-                            if (c == nComp - 1)
+                            if( c == nComp - 1 )
@@ -556 +556 @@ struct SteppingAction : G4UserSteppingAction
-                                nPhotComp = static_cast<G4int>(fNumPhotons * yield[c] / yieldSum);
+                                nPhotComp = static_cast<G4int>( fNumPhotons * yield[c] / yieldSum );
@@ -559,2 +559,2 @@ struct SteppingAction : G4UserSteppingAction
-                            if (nPhotComp > 0)
-                                U4::CollectGenstep_DsG4Scintillation_r4695(aTrack, aStep, nPhotComp, c + 1, tc[c]);
+                            if( nPhotComp > 0 )
+                                U4::CollectGenstep_DsG4Scintillation_r4695( aTrack, aStep, nPhotComp, c + 1, tc[c] );

Have any feedback or feature suggestions? Share it here.

src/GPURaytrace.h

outdated suggestion

github-actions

Cpp-linter Review

Used clang-format v20.1.2

Click here for the full clang-format patch

diff --git a/src/GPURaytrace.h b/src/GPURaytrace.h
index 03318df..2a27b24 100644
--- a/src/GPURaytrace.h
+++ b/src/GPURaytrace.h
@@ -527 +527,2 @@ struct SteppingAction : G4UserSteppingAction
-                        const G4int tcKeys[3] = {kSCINTILLATIONTIMECONSTANT1, kSCINTILLATIONTIMECONSTANT2, kSCINTILLATIONTIMECONSTANT3};
+                        const G4int tcKeys[3] = {kSCINTILLATIONTIMECONSTANT1, kSCINTILLATIONTIMECONSTANT2,
+                                                 kSCINTILLATIONTIMECONSTANT3};
@@ -540,3 +541,2 @@ struct SteppingAction : G4UserSteppingAction
-                                yield[c] = MPT->ConstPropertyExists(yieldKeys[c])
-                                               ? MPT->GetConstProperty(yieldKeys[c])
-                                               : (c == 0 ? 1.0 : 0.0);
+                                yield[c] = MPT->ConstPropertyExists(yieldKeys[c]) ? MPT->GetConstProperty(yieldKeys[c])
+                                                                                  : (c == 0 ? 1.0 : 0.0);

Have any feedback or feature suggestions? Share it here.

src/GPURaytrace.h

plexoos · 2026-04-07T21:55:35Z

@copilot Fix all style-only issues reported by the cpp-linter CI job on this PR.

Copilot · 2026-04-07T21:58:09Z

@copilot Fix all style-only issues reported by the cpp-linter CI job on this PR.

Fixed in commit 4ce08ad. Applied clang-format Microsoft style to src/GPURaytrace.h:

Wrapped the tcKeys array initializer that exceeded the line width
Reformatted the ternary yield[c] assignment to the single-line form expected by Microsoft style

outdated suggestion

plexoos · 2026-04-07T22:09:01Z

@ggalgoczi this diff does not look like style-only change

plexoos · 2026-04-07T22:09:37Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4ce08ad35b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-07T22:15:33Z

src/GPURaytrace.h

+                            if (c == nComp - 1)
+                                nPhotComp = nRemaining; // last component gets remainder
+                            else
+                                nPhotComp = static_cast<G4int>(fNumPhotons * yield[c] / yieldSum);


Match Geant4 split for third scintillation component

When nComp == 3, this branch gives all rounding remainder to component 3 (nPhotComp = nRemaining), but Geant4’s G4Scintillation::PostStepDoIt computes component 3 as yield3/sum * fNumPhotons (no remainder redistribution). That means the GPU gensteps can overpopulate the slow component (especially for low fNumPhotons, e.g. 1–2 photons/step), which breaks CPU/GPU agreement for 3-component scintillation timing in this path.

Useful? React with 👍 / 👎.

Previously only SCINTILLATIONTIMECONSTANT1 (fast, 7ns) was used for all scintillation photons, missing the slow component (1400ns, 25% yield). This caused GPU to produce zero hits with arrival time >100ns while G4 consistently showed 20% of hits in the 100-11000ns range. Now reads up to 3 scintillation components from the material properties (SCINTILLATIONTIMECONSTANT1/2/3, SCINTILLATIONYIELD1/2/3), splits the photon count proportionally, and creates separate gensteps for each component with its own time constant. Validated: full-distribution KS p=0.84 (GPU vs G4), tail fraction GPU 18.5% vs G4 20.4%, max time GPU 7834ns vs G4 7511ns.

Agent-Logs-Url: https://github.com/BNLNPPS/eic-opticks/sessions/7d338b72-d2c1-427f-916a-81051c4d4b5c Co-authored-by: plexoos <5005079+plexoos@users.noreply.github.com>

ggalgoczi requested a review from plexoos April 4, 2026 02:55

github-actions bot previously requested changes Apr 4, 2026

View reviewed changes

ggalgoczi mentioned this pull request Apr 4, 2026

Wavelength shifting #262

Draft

github-actions bot previously requested changes Apr 7, 2026

View reviewed changes

src/GPURaytrace.h Outdated Show resolved Hide resolved

src/GPURaytrace.h Outdated Show resolved Hide resolved

Copilot started work on behalf of plexoos April 7, 2026 21:55 View session

Copilot AI changed the title ~~fix scintillation genstep collection to include all time components~~ style: fix clang-format violations in src/GPURaytrace.h Apr 7, 2026

Copilot finished work on behalf of plexoos April 7, 2026 21:58

chatgpt-codex-connector bot reviewed Apr 7, 2026

View reviewed changes

ggalgoczi and others added 2 commits April 7, 2026 21:01

style: fix clang-format violations in src/GPURaytrace.h

4ae02fb

Agent-Logs-Url: https://github.com/BNLNPPS/eic-opticks/sessions/7d338b72-d2c1-427f-916a-81051c4d4b5c Co-authored-by: plexoos <5005079+plexoos@users.noreply.github.com>

plexoos force-pushed the fix/scintillation-time-components branch from 4ce08ad to 4ae02fb Compare April 8, 2026 01:02

Conversation

ggalgoczi commented Apr 4, 2026 • edited by Copilot AI Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Cpp-linter Review

Used clang-format v20.1.2

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Cpp-linter Review

Used clang-format v20.1.2

Uh oh!

Uh oh!

Uh oh!

plexoos commented Apr 7, 2026

Uh oh!

Copilot AI commented Apr 7, 2026

Uh oh!

plexoos commented Apr 7, 2026

Uh oh!

plexoos commented Apr 7, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ggalgoczi commented Apr 4, 2026 •

edited by Copilot AI

Loading