Skip to content

Caloskimmer simplify - #4501

Merged
pinkenburg merged 3 commits into
sPHENIX-Collaboration:masterfrom
pinkenburg:caloskimmer-simplify
Oct 26, 2026
Merged

pinkenburg merged 3 commits into
sPHENIX-Collaboration:masterfrom
pinkenburg:caloskimmer-simplify

Conversation

@pinkenburg

@pinkenburg pinkenburg commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • [ X] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work for users)
  • Requiring change in macros repository (Please provide links to the macros pull request in the last section)
  • I am a member of GitHub organization of sPHENIX Collaboration, EIC, or ECCE (contact Chris Pinkenburg to join)

What kind of change does this PR introduce? (Bug fix, feature, ...)

This PR removes the obsolete CaloPacketContainer object from the CaloPacketSkimmer

TODOs (if applicable)

Links to other PRs in macros and calibration repositories (if applicable)

Motivation

Remove the obsolete CaloPacketContainer handling from CaloPacketSkimmer and simplify packet access. The change also follows an “include what you use” approach.

Key changes

  • processDetector checks for a PRDF Event. It reads packets from the event when available, and reads standalone CaloPacket nodes when no PRDF event is available.
  • Non-data PRDF events return ABORTEVENT. Missing or empty packets continue to mark the detector event for discard.
  • Removed container lookup and handling, the set_offlineflag method, and the related private flags.

Potential risk areas

  • The accepted input path changes: standalone CaloPacket nodes are used when no PRDF event exists. Validate this behavior with the intended input formats.
  • A non-data PRDF event returns ABORTEVENT, while a missing PRDF event no longer aborts immediately. This can affect event selection.
  • Test results and current review findings were not supplied. Review severity counts are unavailable.

Possible future improvements

  • Add or update tests for PRDF events, standalone packet nodes, non-data events, and missing or empty packets.
  • Document the supported input formats and event return behavior.

AI-generated summaries can make mistakes. This summary is based on the supplied description and inspected source; use contributor judgment when validating behavior.

@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 074a1b0 and 5ece70b.

Files selected for processing: 2
  • offline/packages/Skimmers/CaloPacket/CaloPacketSkimmer.cc
  • offline/packages/Skimmers/CaloPacket/CaloPacketSkimmer.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.

Walkthrough

CaloPacketSkimmer removes offline-container selection and its associated mode flags. It reads packets from a PRDF event when present, or from standalone CaloPacket nodes when no PRDF event exists.

Changes: Packet input selection

Layer / File(s) Summary
Select and process packet sources
offline/packages/Skimmers/CaloPacket/CaloPacketSkimmer.h, offline/packages/Skimmers/CaloPacket/CaloPacketSkimmer.cc
The skimmer removes set_offlineflag, its associated fields, and offline-container handling. It reads packets from a PRDF event when present, and from standalone CaloPacket nodes otherwise. A non-data PRDF event returns ABORTEVENT; per-packet discard handling remains.

Sequence Diagram(s)

sequenceDiagram
  participant Skimmer as CaloPacketSkimmer
  participant PRDF as PRDF Event
  participant Nodes as standalone CaloPacket nodes
  Skimmer->>PRDF: Check for event
  alt PRDF event exists
    Skimmer->>PRDF: Check event type and read packets
    PRDF-->>Skimmer: Event packets
  else No PRDF event
    Skimmer->>Nodes: Read packets in configured ID range
    Nodes-->>Skimmer: Standalone packets
  end
Loading

Possibly related PRs

Priority: Low

Merge Risk: Minimal · up to 5ece7

The changed packet-selection path has no identified issue requiring a fix before merge.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sphenix-jenkins-ci

Copy link
Copy Markdown

Build & test report

Report for commit 5ece70b743a6dff5df165890a7461af16282bd7a:
Jenkins on fire


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@pinkenburg
pinkenburg merged commit fd705f9 into sPHENIX-Collaboration:master Oct 26, 2026
18 of 22 checks passed
@pinkenburg
pinkenburg deleted the caloskimmer-simplify branch October 26, 2026 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant