Skip to content

Si seed mult - #4402

Open
gregottino wants to merge 2 commits into
sPHENIX-Collaboration:masterfrom
gregottino:si_seed_mult
Open

Si seed mult#4402
gregottino wants to merge 2 commits into
sPHENIX-Collaboration:masterfrom
gregottino:si_seed_mult

Conversation

@gregottino

@gregottino gregottino commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Adding Silicon seed multiplicity to KF Particle. Non breaking change. Requires addition or presence of seed DST in data.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • 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, ...)

Additional output branch for KF Particle trees. Non breaking change.
comment: <> ( What does this PR do? Linking to talk in software meeting encouraged )

TODOs (if applicable)

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

Motivation

Add charged silicon seed multiplicity to KF Particle output trees. This supports studies that require Silicon seed DST data.

Key changes

  • Add getNchargedSiSeedMultiplicity to KFParticle_Tools.
  • Select seeds by bunch crossing and minimum MVTX/INTT cluster counts.
  • Return -1 when the silicon seed container is missing.
  • Add and fill the nSiSeedMultiplicity TTree branch.
  • Initialize the stored multiplicity to -1.

Potential risk areas

  • The new branch changes the KF Particle tree schema.
  • Existing workflows need Silicon seed DST data to obtain valid values.
  • Missing input data produces -1, which downstream analyses must handle.
  • Seed-container iteration adds small event-level processing cost.
  • The change does not appear to modify reconstruction behavior or introduce shared mutable state.

Possible future improvements

  • Document the branch definition and sentinel value.
  • Add tests for missing containers, bunch-crossing selection, and cluster-count thresholds.
  • Assess the branch schema impact on readers and validate performance with representative events.

AI-generated summaries can contain errors. Review the implementation and output behavior before merging.

@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0f05101b-e195-4e0e-8c17-daa114a2327a

📥 Commits

Reviewing files that changed from the base of the PR and between e6c2f74 and 77991a6.

📒 Files selected for processing (4)
  • offline/packages/KFParticle_sPHENIX/KFParticle_Tools.cc
  • offline/packages/KFParticle_sPHENIX/KFParticle_Tools.h
  • offline/packages/KFParticle_sPHENIX/KFParticle_nTuple.cc
  • offline/packages/KFParticle_sPHENIX/KFParticle_nTuple.h

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change adds silicon seed multiplicity counting for a bunch crossing and exports the result as the nSiSeedMultiplicity decay-tree branch.

Changes

Silicon seed multiplicity

Layer / File(s) Summary
Silicon seed counting
offline/packages/KFParticle_sPHENIX/KFParticle_Tools.h, offline/packages/KFParticle_sPHENIX/KFParticle_Tools.cc
Adds getNchargedSiSeedMultiplicity. The method returns -1 when the seed container is unavailable. Otherwise, it counts non-null seeds with the requested bunch crossing and minimum MVTX and INTT cluster counts.
nTuple multiplicity export
offline/packages/KFParticle_sPHENIX/KFParticle_nTuple.h, offline/packages/KFParticle_sPHENIX/KFParticle_nTuple.cc
Adds the initialized multiplicity member, writes the nSiSeedMultiplicity branch, and populates it using the first daughter’s bunch crossing.

Sequence Diagram(s)

sequenceDiagram
  participant KFParticle_nTuple
  participant KFParticle_Tools
  participant SiliconTrackSeedContainer
  participant DecayTree
  KFParticle_nTuple->>KFParticle_Tools: request multiplicity for first daughter's bunch crossing
  KFParticle_Tools->>SiliconTrackSeedContainer: retrieve silicon seeds
  SiliconTrackSeedContainer-->>KFParticle_Tools: return seed collection
  KFParticle_Tools-->>KFParticle_nTuple: return qualifying count or -1
  KFParticle_nTuple->>DecayTree: write nSiSeedMultiplicity
Loading

Merge Risk: ⚪ Minimal · up to 77991

This change adds Silicon seed multiplicity to KF Particle tree output without changing existing behavior; no actionable merge-blocking risk remains after normal checks and review.


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 77991a68da969b951ee5ff8ffee829772656c295:
Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@cdean-github

Copy link
Copy Markdown
Contributor

I think this looks fine to me but I'd put this part behind a verbosity statement:

The intent from what I gather is you look at the silicon seeds for that bunch crossing, count if there's INTT and MVTX or return 0 if there's nothing (or -1 for no container). Is requesting that there are MVTX clusters redundant with requiring INTT clusters?

  if (!m_siliconSeeds)
  {
    std::cout << " ERROR: Can't find SiliconTrackSeedContainer " << std::endl;
    return -1;
  }

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.

2 participants