Tracking dev progress#451
Open
HaiwangYu wants to merge 477 commits into
Open
Conversation
- Replace Blob*-keyed map with vector indexed by blob position (determinism) - Hoist scope-transform computation out of per-step path-check inner loops - Replace double-lookup closest_index update with operator[]+std::next erase - Move early return before std::sort (skip wasted sort when num<=1) - Add explicit parentheses to all ||/&& ambiguous conditions; remove pragma - Delete dead scope_name variable and unused num_edges counter - Add review doc: clus/docs/clustering/clustering_protect_overclustering_review.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… fix
The PDVD field response (protodunevd_FR_imbalance3p_260501.json.bz2) had
an all-zero sentinel path at pp=0 on the W plane, under-normalising the
W collection peak by ~12% (peak x1.124, integral x1.117). The deficit
had been absorbed empirically into the per-CRP postgain calibration.
The FR has now been corrected (FR_xn_boost_3.json.bz2 copied over the
same filename in wire-cell-data); de-compensate the postgain accordingly.
- params.jsonnet: bottom 1.1365 -> 1.0 (PDHD-equivalent; same chip);
top 1.52 -> 1.36 (= 1.52 / 1.117).
- chndb-resp-{bot,top}.jsonnet: comments only. The kernel arrays are
intentionally NOT regenerated -- NF thresholds were tuned against
these response shapes, and re-tuning is deferred to a future NF
re-calibration pass. Header notes record the generation-time vs
current postgain so the discrepancy is visible to readers.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a per-sub-window post-trigger veto in decide_trigger() that
rejects fired sub-windows looking like real prolonged tracks rather
than L1SP unipolar lobes. Targets the residual prolonged-track FP
class observed against pdvd/sp_plot/handscan_039324_anode0.csv on
PDVD bottom anode 0 (evt 0 U 82-87, 89-91, 96; evt 3 U 319-331).
The veto is OFF by C++ default (m_l1_pdvd_track_veto_enable{false})
so PDHD's decide_trigger is bit-identical to before this change.
Enabled in cfg/pgrapher/experiment/protodunevd/sp.jsonnet for bottom
anodes only (n < 4); top anodes inherit the C++ defaults pending
their own hand-scan validation. Also adds four PDVD-bottom
threshold overrides on the existing knobs (l1_len_long_mod 100→180,
l1_len_fill_shape 50→90, l1_fill_shape_fill_thr 0.38→0.30,
l1_fill_shape_fwhm_thr 0.30→0.25) tuned in the same campaign.
Aggregate per-ROI metrics on the PDVD bottom anode 0 NPZ dumps:
F1 0.833 (PDHD defaults) → 0.862 (4 threshold tweaks alone) →
0.923 (with veto enabled). All 7 prolonged-track fires the user
flagged in the hand-scan are silenced; 0 GT-positive hits lost.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…gh coverage
The negative-polarity branch loads neg-half(bipolar) as basis1, whose
trough sits at native +12 µs (PDVD V) / +10 µs (PDHD V). The original
build_G window (−15 µs, +10 µs) clipped this trough — partly for PDHD V
(17 in-window samples), entirely for PDVD-bottom V (0 samples). PDVD
ROIs like 039324:2 ch987_t2537_neg returned empty LASSO fits.
Fix: keep `basis1_offset = 0` for the negative case (preserving the
"β fires before the raw trough" geometry that PDHD already had) and
widen `t_hi` to +15 µs for negative polarity only. pad_R is bumped to
30 ticks for negative ROIs to keep boundary β's seeing the full kernel.
Verified across PDHD run 27409 (all events, APA1) and PDVD-bottom run
39324 (all events, anode 0): median(smear_peak − raw_min) per plane now
matches the kernel-geometry prediction `−(t_trough − frame_origin)/0.5`
within 1 tick on every plane (PDHD U=0/expected +1; PDHD V=−4/expected
−3.6; PDVD-bot U=+2/expected +2.1; PDVD-bot V=−4/expected −3.3).
PDVD-bottom V ch987_t2537_neg now produces a non-empty LASSO fit.
Also in this commit (carried-over from in-flight work in this branch):
* L1SPFilterPD: zero the ROI when LASSO declines (sum_beta below
threshold) so no decon passes through.
* protodunevd/sp.jsonnet: cap PDVD top anodes (ident ≥ 4) to 'dump'
mode (electronics not yet validated for L1SP fit) and bump PDVD
bottom peak/mean post-smearing thresholds.
Doc updates in sigproc/docs/l1sp/{README,L1SPFilterPD}.md describe the
asymmetric build_G window and pad_R.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…re-ROI)
Add a special-mode dump of the deconvolved waveform per channel
immediately after FR/ER division but BEFORE the wire-pitch Wire_ind /
Wire_col filter and BEFORE any later software filter (Wiener_*,
Gaus_wide, ROI_*_lf) or ROI mask. Wired through magdecon as
h{u,v,w}_rawdecon<ident> for offline software-filter tuning.
The tap is OFF by default — production runs are bit-identical. Enable
via OmnibusSigProc config rawdecon_tag (default empty) or its TLA
proxy dump_rawdecon=true (added in pdhd/protodunevd sp.jsonnet).
The tag is also added to the rawsigmerge / l1spfinal FrameMergers'
mergemap entries so the trace flows through the L1SP-bypass pipeline
without being dropped.
Implementation
- OmnibusSigProc.h: add m_rawdecon_tag (default ""), m_rawdecon_r_data[3]
- OmnibusSigProc.cxx::decon_2D_init(): conditionally copy m_c_data
after FR/ER division, then replay the IFFT + wire-shift + time-shift
+ unpad path on the saved spectrum into m_rawdecon_r_data; the
swap-with-m_r_data trick reuses unpad_data() unchanged.
- OmnibusSigProc.cxx::process(): after decon_2D_init, save the buffer
via save_data() with the rawdecon tag (before ROI/filter cascade
overwrites m_r_data), tag traces on the output frame.
Configuration
- protodunevd/sp.jsonnet, pdhd/sp.jsonnet: thread dump_rawdecon
through make_sigproc → OmnibusSigProc.rawdecon_tag, plus mergemap
entries on rawsigmerge and l1spfinal so the tag survives the L1SP
bypass merger.
- protodunevd/magnify-sinks.jsonnet, pdhd/magnify-sinks.jsonnet:
add 'rawdecon%d' to magdecon's frames list (silently skipped when
the input frame doesn't carry the tag, so production-mode is
unaffected).
Verification
- Production-mode regression (-R off, dump_rawdecon=false): NF+SP+
sp_to_magnify on PDHD APA1 evt0 produces identical W-plane TH2s
bit-for-bit vs a baseline-equivalent build with these changes
stashed. Existing U/V drift between baseline and current toolkit
is pre-existing (unrelated to this commit).
- Special-mode (-R on, dump_rawdecon=true): all 12 magnify files
(PDHD APA0-3 evt0 of run 27409, PDVD anode0-7 evt0 of run 39324)
now contain h{u,v,w}_rawdecon<ident> TH2s of the expected shapes
with non-trivial signal where _gauss shows tracks.
Used by pdvd/sp_plot/filter_tune_viewer.py (see
github.com:WireCell/wcp-porting-validation.git) for live tuning of
Wire / HF / LF software filters on top of the bare deconvolution.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
decon_2D_init() now slices m_rawdecon_r_data[plane] down to the same unpadded grid that production wiener/gauss live on. Before this fix the rawdecon TH2 was emitted on the padded FFT grid (m_fft_nwires x m_fft_nticks), causing channel/tick offsets vs the companion wiener/gauss histograms and breaking the filter_tune_viewer offline-filter overlay everywhere except files re-emitted with the fix in place. Cosmetic comment cleanup in pdhd/sp.jsonnet, protodunevd/sp.jsonnet, protodunevd/magnify-sinks.jsonnet. No behaviour change there. Verified: regenerating all 12 magnify files (PDVD evt0 anodes 0-7, PDHD evt0 APAs 0-3) with the fix gives offline-filter / production- wiener peak alignment for every anode in the filter_tune_viewer. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tighten the loose LF filter on PDVD top anodes (4-7) slightly for improved baseline suppression. PDVD bottom (0-3) and PDHD remain at 0.002 MHz. Top and bottom instances are already separate objects (split in 9f0179f); this is the first parameter divergence between them. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The old loop iterated rois_*_loose directly while calling BreakROI(), which erases elements from the same list and deletes the SignalROI objects. This produced iterator invalidation (UB) and use-after-free on the pointers pushed into all_rois, followed by a second BreakROI1 pass over the freed objects. Fix: snapshot the list into a local vector before iterating, so BreakROI mutates a separate container. Drop the all_rois / BreakROI1 second pass entirely — those pointers pointed to already-freed objects, so the second pass was reading freed memory with undefined observable effect. This was the empirically-identified source of PDVD SP run-to-run non-determinism on a subset of channels. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…dump When dump_path is set in the config (default ""), ShieldCouplingSub writes one NPZ file per top-U group containing: channels, strip_lengths, wf_in_raw, wf_in_norm, signal_mask, medians_norm, wf_out_raw This lets the shield_tune_viewer Bokeh app reconstruct and visualize the full per-tick median subtraction pipeline (signal masking, correction image, before/after) without rerunning WCT. Wire it in via run_nf_sp_evt.sh -S <dir> or by passing --tla-str shield_dump_path=<dir> to wct-nf-sp.jsonnet. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…D_looseROI decon_2D_looseROI allocated c_data_afterfilter(m_c_data.rows(), ...) but only populated rows iterated by m_channel_range[plane] (OSP wires 0..m_nwires-1). Rows m_nwires..m_fft_nwires-1 were left UNINITIALIZED; inv_c2r propagated heap garbage row-wise, and the m_pad_nwires-offset .block() extract pulled it into the LAST m_pad_nwires rows of m_r_data (OSP wires m_nwires-m_pad_nwires..m_nwires-1). On PDVD V plane that's exactly OSP wires 465-475 = WCT idents 1228-1238 = frame rows 752-762, matching the empirically observed non-determinism band: 12 fresh runs of the PDVD anode 0 fixture clustered into 3 distinct frame_wiener0 states across (5, 4, 3 runs) with max|d|=692 ADC. PDHD APA1 was bit-deterministic across runs but the same uninitialized-memory path was active, leaking into the same band of wires. Why this only surfaced as non-determinism on PDVD: PDVD bottom-drift anodes have more aggressive memory pressure (additional Resampler stage, longer FFT lengths) so the heap state at decon_2D_looseROI's allocation differs across processes; PDHD's smaller pipeline tends to land the same garbage values. Fix mirrors the pattern used in decon_2D_tightROI / decon_2D_ROI_refine / decon_2D_charge: initialize all rows with the default filter, then apply the per-channel bad/lf_noisy override in a second pass. After fix: 12 fresh runs are bit-identical for both l1sp_pd_mode='process' and l1sp_pd_mode=''. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The frozen reference at sigproc/test/data/protodunehd-sp-frames-anode1.tar.bz2 was added at commit ae43013 and predates four legitimate algorithmic changes that have since changed PDHD APA1 SP output: - 14b6c6d: 'rawdecon' tap support in OmnibusSigProc - b4d6198: rawdecon trim to (m_nwires x m_nticks) grid - dc61376: BreakROIs UAF fix in ROI_refinement (snapshot pattern; changes ROI iteration order on every plane, not just PDVD) - 36489a2: decon_2D_looseROI uninitialized FFT-padding rows fix Per-channel RMS rolled forward step-by-step on this branch: ae43013 → 14b6c6d: 34/2560 ch >1% (max rel 49%) 14b6c6d → dc61376: 1010/2560 ch >1% (max rel 235%) dc61376 → 36489a2: 1019/2560 ch >1% (max rel 235%) Reference regenerated at HEAD. bats test now passes against the new reference. No algorithmic change in this commit; only the in-tree fixture is updated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
End-to-end test against the production wct-nf-sp.jsonnet on the PDVD bottom-drift anode 0 raw fixture for run 039324 evt 0. Compares SP output (gauss0, wiener0) bit-exactly against a frozen reference at sigproc/test/data/protodunevd-sp-frames-anode0.tar.bz2. Differences from the PDHD APA1 test: - reality='data' inserts the 512->500 ns Resampler before NF - l1sp_pd_mode='process' enables LASSO writeback - No elecGain override; protodunevd/params.jsonnet carries gains SP output is bit-deterministic on this branch after two fixes: - dc61376: BreakROIs UAF / iterator UB (snapshot-then-iterate) - 36489a2: decon_2D_looseROI uninitialized FFT-padding rows Reference is force-added (sigproc/.gitignore excludes *.bz2 by default, matching the pattern used for sigproc/test/data/protodunehd-sp-frames-anode1.tar.bz2). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… refresh
Updates L1SPFilterPD.md's regression-test section with:
- PDHD reference fixture refresh history (now 2026-05-06 baseline,
incorporates dc61376 + 14b6c6d + b4d6198 + 36489a2).
- New "End-to-end PDVD anode 0 regression (BATS)" section covering
check_pdvd_anode0_nf_sp.bats: the PDVD bottom-drift counterpart
to the PDHD APA1 test, the two SP-side fixes (BreakROIs UAF +
decon_2D_looseROI uninitialized FFT-padding rows) needed to reach
bit-determinism on this branch, the indexing convention that maps
OSP wire index → WCT channel ident → frame row, and skip semantics.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ttom and PDHD Match PDVD bottom and PDHD loose LF filter tau to PDVD top (0.003 MHz). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…0273 Commit 41e0273 commented out gain : 14*mV/fC and resolution: 12 in cfg/pgrapher/common/params.jsonnet ("read from fcl"), but PDSP's params.jsonnet only adds postgain/shaping overrides on top of super.elec, so standalone (non-fcl) PDSP runs had no gain/resolution to read. Add the PDSP-specific defaults to its own params.jsonnet to keep the common "read from fcl" intent and restore standalone behavior to its pre-41e02736 state. Also drop the now-unused WireCellPytorch entry from check_pdsp_sim_sp.jsonnet's plugins list. Commit fe32bee disabled the dnnroi() call ("disable dnn by default") but left the plugin load active, so check_pdsp_sim_sp.bats failed on builds without WireCellPytorch. The test config no longer instantiates any Torch component, so the load is dead. bats sigproc/test/check_pdsp_sim_sp.bats now passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Commit d2f3c00 raised the loose LF filter tau from 0.002 to 0.003 MHz for PDHD and PDVD bottom, which shifts the SP gauss/wiener output and broke check_pdhd_apa1_nf_sp.bats (72/2560 channels >1% RMS) and check_pdvd_anode0_nf_sp.bats (214 rows not bit-exact). Re-record both reference fixtures from HEAD using the same wire-cell commands the bats tests run. Both tests pass against the new references. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous commit dc61376 disabled the BreakROI1 second pass in BreakROIs under the premise that BreakROI erases its argument from rois_*_loose and deletes the SignalROI, making the post-iteration BreakROI1 a use-after-free. That premise is inverted: - BreakROI (line ~1872) only mutates roi->get_contents() in place. No rois_*_loose erase, no delete on the roi argument. - BreakROI1 (line ~2337) is the function that erases from rois_*_loose (lines ~2387-2398) and deletes the old SignalROI (line ~2450). It split each loose ROI at near-zero crossings into sub-ROIs and relinked the front_rois / back_rois / contained_rois maps. The original two-phase pattern (collect into all_rois during a list-safe BreakROI traversal, then call BreakROI1 in a separate loop after the iteration is done) was deliberate: it deferred the list-mutating call to a phase where iteration over the list was already complete. No UB. The actual root cause of the PDVD anode 0 SP non-determinism that motivated dc61376 was uninitialized FFT-padding rows in OmnibusSigProc::decon_2D_looseROI, fixed independently in commit 36489a2. That fix alone is sufficient for bit-determinism. This commit: - Restores the all_rois collection and the BreakROI1 post-pass. - Keeps the snapshot-then-iterate pattern in the BreakROI loop. It is harmless (BreakROI does not mutate the list) but defensive against future maintenance, and it is what was already there. - Refreshes sigproc/test/data/protodunehd-sp-frames-anode1.tar.bz2 and protodunevd-sp-frames-anode0.tar.bz2 because BreakROI1 materially changes SP output. - Updates sigproc/docs/l1sp/L1SPFilterPD.md to reflect the revert and the corrected attribution of the bit-determinism fix. All three sigproc bats tests pass at the new tip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both regression tests previously ran wire-cell against the live pdvd/wct-nf-sp.jsonnet / pdhd/wct-nf-sp.jsonnet entry points. Any edit to those files or their imports (sp.jsonnet, params.jsonnet, nf.jsonnet, …) would silently change the pipeline and break the bit-exact comparison. Replace with pre-rendered frozen configs committed in sigproc/test/data/: pdvd-anode0-nf-sp-cfg.json — anode 0, reality=data, l1sp_pd_mode=process pdhd-apa1-nf-sp-cfg.json — APA1, reality=data, elecGain=14 To refresh after a deliberate algorithm or config change, re-render with the jsonnet command documented in the bats file header and commit the new JSON alongside the new reference tar.bz2. Also adds !sigproc/test/data/*.json exception to .gitignore (which blanket- ignores *.json) so future refreshes can be committed without -f. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Remove the process→dump auto-downgrade for anodes 4-7 and extend the PDVD-tuned trigger-gate overrides (l1_len_long_mod=180, l1_len_fill_shape=90, fill/fwhm thresholds, track-veto block) to all anodes. Top-CRP uses bottom-tuned values as a starting point pending its own hand-scan validation. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Mirrors check_pdvd_anode0_nf_sp.bats for the top-CRP path (ident >= 4, JsonElecResponse, pdvd_top_l1sp_kernels.json.bz2) using run 039324 evt 0 anode 7. L1SP runs in process mode (the former process->dump auto-downgrade for top anodes was removed in the parent commit). Bit-exactness verified across two independent runs; reference frozen at the current apply-pointcloud tip. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ing item Add "End-to-end PDVD anode 7 regression (BATS)" sub-section documenting check_pdvd_anode7_nf_sp.bats (top-CRP counterpart to the anode-0 test, bit-exact, L1SP process mode, pdvd_top_l1sp_kernels.json.bz2). Remove stale "PDVD jsonnet port" pending-work item — L1SPFilterPD is wired into protodunevd/sp.jsonnet and both bottom and top tests exist. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…atures When dump_all_rois=true (auto-enabled by pdhd/run_nf_sp_evt.sh -w), every in-scope ROI gets its own NPZ — raw/decon/lasso/smeared plus the ~30 AsymRecord scalar features (gmax, core_*, raw_asym_wide, ...) and the heuristic flags (flag, flag_l1, flag_l1_adj, adj_donor_ch). Default remains false: triggered-only output is unchanged. compute_asym now fills the dump-only AsymRecord fields when EITHER m_dump_mode OR m_wf_dump_path is set, so process-mode dumps carry the same feature set the calibration writer produces. Bit-for-bit equality vs calibration mode verified on event 0 of run 027409 (32,629 ROIs, 0 mismatches). dump_roi_waveforms moved from class method to anon-namespace helper (renamed dump_roi_npz) so it can take AsymRecord directly without exporting the struct through the public header. cfg/pdhd/sp.jsonnet: pass-through l1sp_pd_dump_all_rois into the L1SP data block. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A multi-plane variant of DNNROIFinding for models trained on more
than one wire plane stacked along the channel axis. Motivation:
the local PDHD DNN-ROI model in lastgeorge/DNN_ROI_SP is trained
on APA0 U+V combined input (1, 3, 1600, 1500), incompatible with
the per-plane DNNROIFinding.
New config keys vs the single-plane node:
- planes: [int] input planes to stack
- output_planes: [int] indices into planes whose model output is
emitted (empty = all)
- outtags: [string] one per output_plane
- debugfile: string optional per-call pickle dump of
(input, output, meta) for offline replay
PDHD wiring in cfg/pgrapher/experiment/pdhd/dnnroi_mp.jsonnet:
APA0 feeds U+V to the model but only emits U; V uses standard SP
gauss via PlaneSelector. APA1-3 emit both U and V. W is always
passthrough (model has no W head).
Architecture doc updated. Build requires libtorch (CUDA optional).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both DNNROIFinding and DNNROIFindingMultiPlane hardcoded all internal
arrays — and the output trace length — at m_ncols = m_cfg.nticks
(jsonnet config). On PDHD the SP frame is 5999 ticks but nticks=6000
in jsonnet, so the C++ silently zero-padded the input to 6000 and
emitted 6000-tick output traces. Worse, an input frame *larger* than
nticks would have been silently truncated.
Make the per-call sizing input-driven instead:
- Probe input_ticks from the first non-empty input trace.
- Compute model_ticks = ceil(input_ticks / tick_per_slice) *
tick_per_slice (smallest multiple of tps covering the input).
- Size all eigen arrays at (nchan, model_ticks); zero-pad the
trailing model_ticks - input_ticks columns.
- Downsample / model / upsample / mask at model_ticks width
(always exactly divisible).
- Crop sp_charge to input_ticks columns before constructing output
traces.
eigen_to_traces now uses arr.cols() instead of m_ncols, so the
cropped width flows through naturally. m_cfg.nticks stays as a
documented expected size and a fallback default (degenerate
empty-input case). Debug-level log line per call:
input_ticks=… model_ticks=… cfg.nticks=…
Info-level warning on first call if input_ticks > cfg.nticks (silent
override visible).
Verified on PDHD run 27409 / event 0 / APA0:
- SP gauss frame shape (2560, 5999)
- DNN dnnsp0 frame shape (2560, 5999) (was (2560, 6000) before)
- Kept range values bit-identical to pre-fix output (dropped
trailing column was always zero by construction).
- Works in both -m pp and -m mp wirings.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New per-anode subgraph that mirrors dnnroi_mp.jsonnet but swaps the single DNNROIFindingMultiPlane(planes=[0,1]) node for two stock single-plane DNNROIFinding(plane: 0/1) nodes sharing the same TorchService. Each call to the model is (1, 3, 800, 1500) instead of the stacked (1, 3, 1600, 1500); the two calls are serialized by the TorchService semaphore so the peak activation footprint is for one plane at a time — roughly half the stacked-plane peak. Per-anode policy unchanged: APA0: U via model; V and W from standard SP gauss (passthrough). APA1-3: U and V via model; W from standard SP gauss. Works because the MobileNetV3-Large UNet is fully convolutional on the channel axis — the same CP43.ts (traced at 1600 channels) accepts 800-channel input. Verified empirically by DNN_ROI_SP/scripts/test_per_plane_ts.py. No retrain, no re-trace. Output trace tags, fanout/fanin/retag plumbing, and SP-debug-mode input expectations are identical to dnnroi_mp.jsonnet. Selected at the top-level config via TLA dnnroi_mode='pp'|'mp' (see matching wct-nf-sp-dnnroi.jsonnet edit in wcp-porting-validation). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
New per-anode envelope that wires L1SPFilterPD AFTER a DNN-ROI subgraph,
feeding the DNN output as the L1SP signal channel. Mirrors the standard
L1SP envelope in sp.jsonnet (FrameSplitter ×2 + FrameMerger ×2 +
L1SPFilterPD) but inserts SP + DNN-ROI + Retagger on the signal-side
branch instead of bare OmnibusSigProc.
Architecture per anode:
post-NF frame --FrameSplitter--> [0] SP -> DNN-ROI -> Retagger(dnnsp%d
-> gauss%d & wiener%d)
[1] (raw%d preserved) -----------------+
v
FrameMerger
{ raw, gauss, wiener }
v
FrameSplitter
/ \
L1SPFilterPD ----+
(adctag=raw, |
sigtag=gauss, v
outtag=gauss) -> FrameMerger
L1SP gauss -> gauss & wiener
raw passthrough
The pre-SP FrameSplitter mirrors sp.jsonnet's rawsplit because
OmnibusSigProc drops raw%d from its output; raw must be branched
before SP and merged back later. The post-merge FrameSplitter
mirrors sp.jsonnet's sigsplit so the same combined frame can feed
both the L1SP node and the final merger.
L1SPFilterPD config block is copy-paste from sp.jsonnet:130-174
(canonical PDHD tuning). Keep the two in sync if either is re-tuned.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The companion change in wcp-porting-validation removes the redundant SP-only tap and raw tap from wct-nf-sp-dnnroi.jsonnet so the DNN-ROI runner now produces a single canonical archive per anode. The envelope's sp_frame_tap optional argument was the hook for piping the SP-only diagnostic tap inside the SP+DNN chain; with the tap gone, the parameter (and its conditional inclusion in pg.pipeline) is dead. Drop the parameter and the branch. No functional change for callers that already passed sp_frame_tap=null (the new default for the top-level config). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
OmnibusSigProc attaches per-channel Wiener-filter thresholds as the
trace_summary of the wiener%d tag (OmnibusSigProc.cxx:1963).
Downstream Magnify duplicates wiener%d into threshold%d via Retagger
(wct-sp-to-magnify.jsonnet) and MagnifySink writes a 1D TH1F named
h[uvw]_threshold0 from the summary.
The DNN-ROI subgraph was dropping that summary:
- DNNROIFinding has a summary_tag config but defaulted to "" (no
summary fetched).
- PlaneSelector (V/W gauss passthroughs) had no summary_tags set.
Result: post-DNN Magnify ROOT files had hu/hv/hw_threshold0 present
by name but with all-zero bin contents.
Fix is two-line:
- dnn_node(): summary_tag: 'wiener%d' % apaid
=> DNNROIFinding.cxx:204 get_summary_e fetches
and remaps the wiener summary by channel onto
the output dnnsp%du / dnnsp%dv trace tag.
- pass_through(): summary_tags: ['wiener%d' % apaid]
=> PlaneSelector.cxx:137-141 pulls the wiener
summary alongside the gauss waveforms and
attaches it to the output dnnsp%dv / dnnsp%dw
trace tag (per PlaneSelector.cxx:54-58 docs).
The merged dnnsp%d trace tag carries the concatenated 2560-channel
threshold summary. Downstream Retagger (rename+merge), FrameMerger
(per-channel summary fallback at FrameMerger.cxx:101-103), L1SPFilterPD
(does not touch wiener), the final FrameMerger, and FrameFileSink all
preserve trace_summary correctly. The Magnify Retagger duplicates
the resulting wiener%d summary into threshold%d and MagnifySink writes
non-zero TH1Fs.
Verified on run 27409 / event 0 / APA0: hu/hv/hw_threshold0 in the
DNN-ROI magnify ROOT file now match the standard SP version bin-for-
bin (sums 145614 / 152935 / 192019, max 229 / 321 / 283).
DNNROIFindingMultiPlane (-m mp legacy path) hardcodes an empty summary
at .cxx:402 and has no summary_tag config; fixing it requires a C++
change and is left out of scope (pp is the production default).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make this new one, as #444 seems not working for the tracking purpose