fix(sim): honor configured record output in production builds by plexoos · Pull Request #387 · BNLNPPS/simphony

plexoos · 2026-06-26T22:09:12Z

This fixes Integration.simg4ox failures in Release builds where PRODUCTION is defined.

The test config uses DebugLite, which requests persisted photon step records, but the Release
build had parts of the record.npy path compiled out behind #ifndef PRODUCTION. As a result, the
Opticks-side A000/record.npy file was not produced even though the runtime config requested it.

This change keeps full photon step records available in Release builds when explicitly configured,
while preserving normal production behavior where records are not allocated or written unless
requested.

Allow sctx::point() to exist in PRODUCTION builds.
Keep only the lightweight evt->record write available in production.
Leave heavier debug streams (rec, prd, aux, seq, tag, flat) guarded as non-production/debug-only.
Allocate evt->record only when SEventConfig::GatherRecord() is true and max_record > 0.
Guard GPU and host ctx.point() calls with evt->record, so normal production configs do not write photon histories.
Allow QEvt to gather/save SCOMP_RECORD in production when requested.
Update the A/B comparison helper to use a Release-specific expected mismatch list for Geant4 11.3.
DebugLite / explicit record request:
- Release builds produce record.npy.
- Integration.simg4ox can compare A/B records.
Minimal / normal production mode:
- No record buffer is allocated.
- No ctx.point() record writes are performed.
- No record.npy is saved.

github-actions · 2026-06-26T22:09:46Z

Cpp-Linter Report ⚠️

Some files did not pass the configured checks!

clang-format (v20.1.2) reports: 1 file(s) not formatted

qudarap/QEvt.cc

Have any feedback or feature suggestions? Share it here.

This fixes `Integration.simg4ox` failures in Release builds where `PRODUCTION` is defined. The test config uses `DebugLite`, which requests persisted photon step records, but the Release build had parts of the `record.npy` path compiled out behind `#ifndef PRODUCTION`. As a result, the Opticks-side `A000/record.npy` file was not produced even though the runtime config requested it. This change keeps full photon step records available in Release builds when explicitly configured, while preserving normal production behavior where records are not allocated or written unless requested. - Allow `sctx::point()` to exist in `PRODUCTION` builds. - Keep only the lightweight `evt->record` write available in production. - Leave heavier debug streams (`rec`, `prd`, `aux`, `seq`, `tag`, `flat`) guarded as non-production/debug-only. - Allocate `evt->record` only when `SEventConfig::GatherRecord()` is true and `max_record > 0`. - Guard GPU and host `ctx.point()` calls with `evt->record`, so normal production configs do not write photon histories. - Allow `QEvt` to gather/save `SCOMP_RECORD` in production when requested. - Update the A/B comparison helper to use a Release-specific expected mismatch list for Geant4 11.3. - `DebugLite` / explicit `record` request: - Release builds produce `record.npy`. - `Integration.simg4ox` can compare A/B records. - `Minimal` / normal production mode: - No record buffer is allocated. - No `ctx.point()` record writes are performed. - No `record.npy` is saved.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4a00c7d732

ℹ️ 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-06-26T22:13:00Z

-#ifndef PRODUCTION
-    ctx.point(bounce);
-#endif
+    if(evt->record) ctx.point(bounce);


Preserve seq-only histories in GPU simulation

In non-PRODUCTION runs using OPTICKS_EVENT_MODE=HitSeq or HitPhotonSeq, SEventConfig::Initialize_Comp_Simulate_ configures SCOMP_SEQ without SCOMP_RECORD, so evt->record is null. sctx::point is also where seq.add_nibble(...) runs, so guarding the whole call here (and the matching post-bounce call) prevents any sequence history from being accumulated; ctx.end() then saves empty/zero seq.npy for seq-only modes.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-06-26T22:13:00Z

    if(first_point == false) ctx.trace(bounce);
-    ctx.point(bounce);  // sseq::add_nibble the current photon flag
 #endif
+    if(evt->record) ctx.point(bounce);  // sseq::add_nibble the current photon flag in non-production


Preserve seq-only histories in host recording

For host-side/non-PRODUCTION recording with seq enabled but record disabled, such as OPTICKS_EVENT_MODE=HitSeq or HitPhotonSeq, evt->record is null while evt->seq is still configured. This guard skips ctx.point, but that method performs the seq.add_nibble(...) update noted in the comment, so the G4/CPU path stops populating sequence histories unless full records are also requested.

Useful? React with 👍 / 👎.

Copilot

Pull request overview

This PR fixes missing record.npy outputs in Release/PRODUCTION builds by keeping full step-record generation and gathering available when explicitly configured, while keeping heavier debug-only streams guarded in production. It also updates the A/B record comparison helper to use build-type-specific expected mismatch lists for Geant4 11.3.

Changes:

Make sctx::point() available in PRODUCTION builds and gate record-writing so it only occurs when configured.
Allow QEvt to allocate/gather SCOMP_RECORD in production (when requested via config) and wire it into component gathering.
Update compare_ab.py to choose expected mismatch indices based on detected build type.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
sysrap/SEvt.cc	Changes per-bounce recording call-site gating for `ctx.point()` to support configured record output.
sysrap/sctx.h	Makes `sctx::point()` available in production while keeping heavier debug writes guarded.
qudarap/QEvt.hh	Exposes `gatherRecord()` in production builds.
qudarap/QEvt.cc	Enables `SCOMP_RECORD` gather + conditional device allocation in production when configured.
optiphy/ana/compare_ab.py	Adds build-type detection to select Release-specific expected mismatch indices for comparisons.
CSGOptiX/CSGOptiX7.cu	Gates GPU-side `ctx.point()` calls to avoid record writes unless configured.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

    if(first_point == false) ctx.trace(bounce);
-    ctx.point(bounce);  // sseq::add_nibble the current photon flag
 #endif
+    if(evt->record) ctx.point(bounce);  // sseq::add_nibble the current photon flag in non-production


-#ifndef PRODUCTION
-    ctx.point(bounce);
-#endif
+    if(evt->record) ctx.point(bounce);


-#ifndef PRODUCTION
-        ctx.point(bounce) ;
-#endif
+        if(evt->record) ctx.point(bounce) ;


+    build_type = os.environ.get("SIMPHONY_BUILD_TYPE") or os.environ.get("CMAKE_BUILD_TYPE")
+    if build_type:
+        return build_type
+



-#ifndef PRODUCTION
 /**
 sctx::point : copy current sphoton p into (idx,bounce) entries of evt->record/rec/seq/aux


Copilot AI review requested due to automatic review settings June 26, 2026 22:09

Copilot started reviewing on behalf of plexoos June 26, 2026 22:09 View session

plexoos mentioned this pull request Jun 26, 2026

Investigate Release vs Debug discrepancy in simg4ox A/B record diff list #386

Open

plexoos force-pushed the fix-integration-test-simg4ox-release branch from 4a00c7d to b97f564 Compare June 26, 2026 22:12

chatgpt-codex-connector Bot reviewed Jun 26, 2026

View reviewed changes

Copilot AI reviewed Jun 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(sim): honor configured record output in production builds#387

fix(sim): honor configured record output in production builds#387
plexoos wants to merge 1 commit into
mainfrom
fix-integration-test-simg4ox-release

plexoos commented Jun 26, 2026

Uh oh!

github-actions Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jun 26, 2026

Uh oh!

chatgpt-codex-connector Bot Jun 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

plexoos commented Jun 26, 2026

Uh oh!

github-actions Bot commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Cpp-Linter Report ⚠️

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Jun 26, 2026 •

edited

Loading