Skip to content

Added feature to get "n_good" and "n_evts" by external systems#4132

Merged
pinkenburg merged 4 commits intosPHENIX-Collaboration:masterfrom
silas-gross:PHHerwig
Jan 24, 2026
Merged

Added feature to get "n_good" and "n_evts" by external systems#4132
pinkenburg merged 4 commits intosPHENIX-Collaboration:masterfrom
silas-gross:PHHerwig

Conversation

@silas-gross
Copy link
Copy Markdown
Contributor

@silas-gross silas-gross commented Jan 21, 2026

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, ...)

Add a "getNgood" and "getNevts" function
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)

Summary

Motivation & Context
Provide external systems read-only access to the internal event counters so they can monitor how many events were processed and how many passed the trigger without exposing or modifying internal state.

Key changes

  • Added inline accessors to both HepMC trigger classes:
    • HepMCJetTrigger::getNevts() and ::getNgood()
    • HepMCParticleTrigger::getNevts() and ::getNgood()
  • Moved placement of some private member declarations (no semantic change).
  • Adjusted increment timing for n_evts in both process_event implementations: n_evts is incremented after the event-limit (goal_event_number) check, so events that immediately trigger ABORTEVENT due to reaching the good-event limit are not counted in n_evts.

Potential risk areas

  • IO / data format: none — no serialization or external file format changes.
  • Reconstruction / processing behavior: minimal. The only behavioral change is the timing of n_evts increments; this affects reported counters (n_evts no longer includes events rejected immediately by the event-limit check) but does not change event acceptance logic beyond counter semantics.
  • Thread-safety: the new getters are unsynchronized reads of non-atomic counters. In multi-threaded contexts counters may be read while being updated—consider race conditions for callers relying on consistent snapshots.
  • Performance: negligible; inline getters are trivial and the counter adjustment has no measurable performance impact.

Possible future improvements

  • Make counters atomic or add explicit synchronization if triggers are used concurrently and consistent snapshots are required.
  • Add unit tests validating counter semantics (especially around event-limit behavior).
  • Document exact semantics of n_evts and n_good (when they increment, reset behavior) in code comments or user documentation.
  • Optionally expose additional statistics (e.g., rejected-by-reason counts).

Note: AI-generated summaries can be imperfect—please review the changes in code to confirm details and semantics.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 21, 2026

📝 Walkthrough

Walkthrough

Two HepMC trigger classes gain public accessors for internal event counters; event-counter increment placement is adjusted in both process_event implementations and some private member declarations are reordered.

Changes

Cohort / File(s) Summary
Header: Accessors added
generators/Herwig/HepMCTrigger/HepMCJetTrigger.h, generators/Herwig/HepMCTrigger/HepMCParticleTrigger.h
Added public methods int getNevts() and int getNgood() exposing n_evts and n_good. Private member declaration order changed (moved set_event_limit earlier).
Implementation: event-counting control flow
generators/Herwig/HepMCTrigger/HepMCJetTrigger.cc, generators/Herwig/HepMCTrigger/HepMCParticleTrigger.cc
Moved n_evts++ to occur after the event-limit (ABORTEVENT) check. If the limit is reached and the function returns ABORTEVENT, n_evts is no longer incremented for that call — behavior differs from prior unconditional increment.

Sequence Diagram(s)

✨ Finishing touches
  • 📝 Generate docstrings

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 and usage tips.

@sphenix-jenkins-ci
Copy link
Copy Markdown

Build & test report

Report for commit 74e453377053707818b5e21f0cbaf433349be06b:
Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@sphenix-jenkins-ci
Copy link
Copy Markdown

Build & test report

Report for commit b56ed66c1776e7b31936c2b5a5da349ceab6df0b:
Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@sphenix-jenkins-ci
Copy link
Copy Markdown

Build & test report

Report for commit 18c5a2c56900618bcf9798bb091f064b628994a4:
Jenkins on fire

Build & test report

Report for commit f704495167ff5a179e3b400f49850351b7084714:
Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@pinkenburg pinkenburg merged commit 8b6df96 into sPHENIX-Collaboration:master Jan 24, 2026
2 of 4 checks passed
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