test(ci): add GPU vs G4 wavelength shifting validation test#270
Conversation
|
Please update or rebase the branch, and change the PR title to better reflect the intended change using conventional commit format. |
Test fires 10000 UV photons (350nm) from outside a WLS sphere (r=10mm) through a Rayleigh scattering medium into a detector shell (r=30mm). Compares GPU vs G4: hit count, WLS conversion fraction, shifted wavelength spectrum (chi2 + KS), and arrival time. Geometry: WLS sphere + scattering medium + detector shell. All 5 tests pass with p>0.01 thresholds.
Now that G4 uses exponential WLS time profile (matching GPU), the shifted photon arrival time distributions should agree. Replace the median-only check with a proper KS test comparing GPU and G4 shifted photon time distributions. Also reports std ratio (expect ~1.0) and unshifted transport time for reference.
Remove the wavelength chi2 test (redundant with KS test). Relax significance threshold from p>0.01 to p>0.001 to accommodate minor ICDF texture interpolation differences in WLS emission spectrum sampling between GPU and G4. Validated: 10/10 seeds pass at p>0.001.
Runs test_wavelength_shifting.sh on pull requests, comparing GPU and G4 wavelength shifting physics on the dual-sphere test geometry. Tests hit count, WLS conversion fraction, shifted wavelength spectrum, and arrival time distribution.
Thin 1mm WLS slab geometry for isolating per-pass conversion and TIR light-piping behavior. Includes detailed KS-test wavelength/time comparison script (wls_diagnostic.py) used to confirm GPU ICDF sampling matches G4 and identify MaxBounce truncation as the primary slab hit-count discrepancy.
ggalgoczi
changed the title
ggalgoczi
force-pushed
the
feat/wls-validation-test
branch
from
116a0d1 to
134f27a
Compare
|
@plexoos rebased, ready for review. Please ping me here once you have finished with review. |
|
Test failed because needs #269 to be merged first |
|
It would make sense to move this test to #269. What is the difference between wls_test.gdml and wls_scatter_viz.gdml? wls_test.gdml does not appear to be used. Can we keep only one test geometry? |
G4OpticalPhysics defaults to delta time profile for G4OpWLS, which applies a fixed delay equal to WLSTIMECONSTANT. The physically correct model is exponential decay sampling: dt = -WLSTIMECONSTANT * log(u). The GPU implementation already uses exponential. This fix aligns G4 with GPU and with the correct stochastic decay physics.
|
@plexoos please let me know if you are done with the review of this PR. |
There was a problem hiding this comment.
Can we reuse the existing wls_test.json? The number of photons can be increased there as needed
There was a problem hiding this comment.
Can we reuse the existing wls_test.json instead of introducing another config?
There was a problem hiding this comment.
this geometry is not used in test_wavelength_shifting.sh
Can we reuse wls_test.gdml? Is there any reason not to use the geometry with spheres for the test?
|
|
||
| # --- GPU run --- | ||
| echo "[GPU] Running GPUPhotonSourceMinimal..." | ||
| GPU_OUT=$(/opt/eic-opticks/bin/GPUPhotonSourceMinimal \ |
There was a problem hiding this comment.
Why hard code the full path to the binary?
|
|
||
| # --- G4 run --- | ||
| echo "[G4] Running StandAloneGeant4Validation..." | ||
| G4_OUT=$(/opt/eic-opticks/bin/StandAloneGeant4Validation \ |
There was a problem hiding this comment.
this executable is not available in this PR. The test fails
| GPU_HITS=$(echo "$GPU_OUT" | grep "Opticks: NumHits" | head -1 | awk '{print $NF}') | ||
| echo "[GPU] Hits: $GPU_HITS" | ||
|
|
||
| GPU_HIT_FILE="/tmp/MISSING_USER/opticks/GEOM/GEOM/GPUPhotonSourceMinimal/ALL0_no_opticks_event_name/A000/hit.npy" |
There was a problem hiding this comment.
This should be a script parameter rather than a hardcoded path
There was a problem hiding this comment.
Why are you changing this file in this PR if it is not used in the test?
| echo "[COMPARE] Analyzing wavelength and time distributions..." | ||
| echo "" | ||
|
|
||
| python3 - "$GPU_HIT_FILE" "$G4_HIT_FILE" "$GPU_HITS" "$G4_HITS" << 'PYEOF' |
There was a problem hiding this comment.
Embedding that much python code in a heredoc becomes a maintainability smell
- Keep the shell script for running binaries and collecting file paths
- Move the comparison logic into a standalone Python script such as tests/wls_compare.py or extend optiphy/ana/wls_diagnostic.py
These three files are not referenced by test_wavelength_shifting.sh or any other test/script. wls_slab.gdml and wls_slab.json appear to be exploratory work that was never wired up; wls_100k.json is a duplicate of config/wls_test.json with numphoton bumped to 100000, which can be overridden at runtime when needed. Removes 173 lines from the PR diff with no behavioural impact.
The SetWLSTimeProfile("exponential") call was added in this PR's
test branch, but GPURaytrace is not used by test_wavelength_shifting.sh
(which runs GPUPhotonSourceMinimal for the GPU side and
StandAloneGeant4Validation for the G4 side). The G4 reference run is
already configured in StandAloneGeant4Validation, so this change is
out of scope for the test PR.
If GPURaytrace also needs the same fix, that belongs in a separate
change.
PR #269 added tests/geom/wls_test.gdml and config/wls_test.json specifically for WLS validation, but they were not used by any test. This PR was originally adding a second WLS geometry (wls_scatter_viz) with a Rayleigh-scattering medium. The scattering is incidental to a WLS-physics validation, so consolidate onto the simpler wls_test geometry already on main and drop the duplicate assets. The four physics tests (hit count, WLS fraction, shifted-wavelength KS, shifted-time KS) remain meaningful: with WLSABSLENGTH=0.1mm at 350nm in a 20mm sphere, both GPU and G4 should produce >99% shifted photons, and the KS tests on the shifted-photon populations are unaffected.
…ise hit paths Three corrections to test_wavelength_shifting.sh: 1. The GPU hit-file path was a literal "/tmp/MISSING_USER/opticks/GEOM/GEOM/...". "MISSING_USER" is opticks' fallback when $USER is unset (sysrap/spath.h:294), and the trailing "GEOM" is the unset $GEOM env var. The script never exported either, so the GPUPhotonSourceMinimal run wrote to a different path than the one the script then loaded. Set USER=fakeuser and GEOM=fakegeom (matching tests/test_GPURaytrace.sh and tests/test_GPUPhotonSource_8x8SiPM.sh) and reference the path through the env vars. 2. Drop the hard-coded "/opt/eic-opticks/bin/" prefix on both binaries. The install dir is on $PATH in the CI image and in the local eic-opticks-env.sh, matching the convention of every other test. 3. StandAloneGeant4Validation is provided by PR #271, not by anything currently on main. SKIP cleanly rather than failing when the binary isn't installed, so this test is benign on branches where #271 has not yet landed. Also pre-clean stale GPU/G4 hit files at the top of the run so a silently-failing binary cannot make the comparison succeed against leftover data.
The previous version embedded ~190 lines of Python in a heredoc inside test_wavelength_shifting.sh, including a hand-rolled `ks_test` that duplicated (with a slightly cruder p-value approximation) the existing ks_test_2sample helper in optiphy/ana/wls_diagnostic.py. Move the four pass/fail checks (hit count, WLS fraction, shifted wavelength KS, shifted time KS) into a standalone script that imports ks_test_2sample from wls_diagnostic, and have the shell script invoke it as `python3 optiphy/ana/wls_test.py <gpu_hit> <g4_hit>`. The standalone form is testable in isolation and lets a developer rerun the comparison against any (gpu hit.npy, g4 hit.npy) pair without re-running the binaries. Behaviour-preserving: thresholds (Z<5 sigma, 3% fraction tolerance, KS p > 0.001) match the heredoc.
|
Thanks, pushed five commits addressing all eight inline comments and issue-comment 4. The three unused assets ( |
plexoos
left a comment
plexoos
left a comment
There was a problem hiding this comment.
Thank you for addressing the comments. LGTM
2 participants
Add automated WLS validation test comparing GPU and G4 on a dual-sphere geometry (WLS sphere + detector shell + scattering medium). Tests hit count (Z-score), WLS fraction (3% tolerance), shifted wavelength (KS p>0.001), and shifted arrival time (KS p>0.001). Passes across 10 different random seeds. Adds test_wavelength_shifting.sh to the CI pipeline in build-pull-request.yaml. Uses GPUPhotonSourceMinimal for GPU and StandAloneGeant4Validation for G4.