Repository navigation
Remove calopacketcontainer trigger - #4502
pinkenburg merged 4 commits into
Conversation
Walkthrough
Priority: Merge Risk:
 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
-
-
- Source mode: Auto
- Repository: sPHENIX-Collaboration/coresoftware
- Branch: master
- File: .coderabbit.yaml
- Commit: fd705f9
- Layers: Repository YAML
- Review profile: CHILL
- Plan: Advanced
- Run ID:
075f5b77-5331-48a5-a815-4e105b50f043
-
offline/packages/trigger/CaloTriggerEmulator.ccoffline/packages/trigger/CaloTriggerEmulator.hoffline/packages/trigger/MBDTriggerEmulator.ccoffline/packages/trigger/MBDTriggerEmulator.h
-
- offline/packages/trigger/CaloTriggerEmulator.h
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
-
| } | ||
|
|
||
| if (!m_waveforms_mbd) | ||
| if (!dstp[0] && !dstp[1]) |
There was a problem hiding this comment.
🔎 Supported by static analysis
-
Script executed:
git diff fd705f9f2368d9e22a5154df23f0d563491bbe53..cc2e50635f0f18a1ec9e880e4a7d4ce9be518602 -- offline/packages/trigger/MBDTriggerEmulator.cc
Repository: sPHENIX-Collaboration/coresoftware
Length of output: 4050
Script executed:
sed -n '270,460p' offline/packages/trigger/MBDTriggerEmulator.ccRepository: sPHENIX-Collaboration/coresoftware
Length of output: 5129
Script executed:
set -e file=offline/packages/trigger/MBDTriggerEmulator.cc printf '%s\n' '--- declarations, event flow, GetNodes ---' rg -n -F -- 'dstp' "$file" rg -n -F -e 'GetNodes(' -e 'process_event(' -e 'process_waveforms(' -e 'process_primitives(' "$file" sed -n '1,180p' "$file" sed -n '180,275p' "$file" sed -n '450,620p' "$file" sed -n '760,835p' "$file" printf '%s\n' '--- base process and declarations ---' git show fd705f9f2368d9e22a5154df23f0d563491bbe53:"$file" | sed -n '1,180p' git show fd705f9f2368d9e22a5154df23f0d563491bbe53:"$file" | sed -n '180,275p' git show fd705f9f2368d9e22a5154df23f0d563491bbe53:"$file" | sed -n '450,620p' git show fd705f9f2368d9e22a5154df23f0d563491bbe53:"$file" | sed -n '760,835p'
Repository: sPHENIX-Collaboration/coresoftware
Length of output: 27867
Script executed:
set -e printf '%s\n' '--- header declarations and pointer initialization ---' sed -n '1,240p' offline/packages/trigger/MBDTriggerEmulator.h printf '%s\n' '--- MBDPackets and packet-node producers/consumers ---' rg -n -F -e 'MBDPackets' -e 'CaloPacket>(topNode' -e 'CaloPacket *' offline framework | head -240 printf '%s\n' '--- relevant producer definitions ---' rg -l -F -e 'MBDPackets' -e 'new CaloPacket' offline framework | while read -r f; do echo "### $f" rg -n -F -C 8 -e 'MBDPackets' -e 'new CaloPacket' "$f" done
Repository: sPHENIX-Collaboration/coresoftware
Length of output: 11418
Script executed:
set -e printf '%s\n' '--- all MBDPackets references ---' rg -n -F --hidden --glob '!*.root' --glob '!build/**' --glob '!dist/**' 'MBDPackets' . printf '%s\n' '--- packet-container construction and insertion ---' rg -n -F --hidden --glob '!*.root' --glob '!build/**' --glob '!dist/**' -e 'CaloPacketContainerv1' -e 'AddPacket(' -e 'new PHIODataNode' -e 'getPacketbyId' .
Repository: sPHENIX-Collaboration/coresoftware
Length of output: 40799
Handle incomplete packet sets before primitive processing.
When only one of packets 1001 or 1002 exists, the current ||-equivalent waveform selection enters process_waveforms(). The missing packet contributes no entries to m_peak_sub_ped_mbd, but process_primitives() unconditionally reads those entries with .at(is), which can throw on a fresh event.
Route partial packet sets to the existing incomplete-event path:
Suggested fix
-
--- "a/offline/packages/trigger/MBDTriggerEmulator.cc" +++ "b/offline/packages/trigger/MBDTriggerEmulator.cc" @@ -291,7 +291,7 @@ std::cout << __FILE__ << "::" << __FUNCTION__ << ":: Processing waveforms" << std::endl; } - if (!dstp[0] && !dstp[1]) + if (!dstp[0] || !dstp[1]) { return process_raw(); }
The individual packet nodes can be created for only the packets present in an event. The pre-PR MBDPackets path supplied both packets before primitive processing, so this PR introduces the partial-set failure. GetNodes() refreshes both pointers on every event, so stale pointers are not the cause.
Committable suggestion
-
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.Suggested changeif (!dstp[0] && !dstp[1]) if (!dstp[0] || !dstp[1])
Build & test reportReport for commit cc2e50635f0f18a1ec9e880e4a7d4ce9be518602:
Automatically generated by sPHENIX Jenkins continuous integration |
0c50905
into
sPHENIX-Collaboration:master
Types of changes
What kind of change does this PR introduce? (Bug fix, feature, ...)
This PR removed the obsolete CaloPacketContainer class from the trigger emulators
TODOs (if applicable)
Links to other PRs in macros and calibration repositories (if applicable)
Motivation
Remove obsolete
CaloPacketContaineruse from the calorimeter and MBD trigger emulators. The change replaces container access with individualCaloPacketlookup. The supplied context states that offline packets must not be deleted.Key changes
Initmethod. Both emulators removem_useoffline.Potential risk areas
Possible future improvements
AI-generated summaries can contain mistakes. Verify these details against the implementation and use scientific judgment before relying on them.