Skip to content

feat(MinimumBiasClassifier): Add PHParameters for detailed cut tracking #3884

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
Steepspace wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

feat(MinimumBiasClassifier): Add PHParameters for detailed cut tracking #3884

Steepspace wants to merge 7 commits into from

Conversation

@Steepspace
Copy link
Contributor

@Steepspace Steepspace commented Sep 19, 2025

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

This PR introduces PHParameters to the MinimumBiasClassifier to provide detailed, event-by-event information about why the minimum bias selection passes or fails. Previously, the module only set a single boolean, making it difficult to debug or perform quality assurance checks.

Key changes include:

  • A PHParameters object named "MinBiasParams" is now created and stored on the PAR node.
  • Individual boolean flags are set to true if an event fails a specific selection criterion, such as:
    • minbias_background_cut_fail
    • minbias_two_hit_min_fail
    • minbias_zdc_energy_min_fail
    • minbias_mbd_total_energy_max_fail
  • Key calculated values (e.g., MBD north/south charge, vertex scale) are also saved to this object for downstream analysis and QA.
  • The total MBD charge cut is no longer a hardcoded constant and can be configured via the new set_mbd_total_charge_cut() method. (Note: The default total MBD charge cut of 2100 remains unchanged.)

TODOs (if applicable)

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

@sphenix-jenkins-ci
Copy link

sphenix-jenkins-ci bot commented Sep 20, 2025

Build & test report

Report for commit 2af2be95566efb8e8f5eb2dda64ab85a79543791:
Jenkins passed

Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@Steepspace Steepspace marked this pull request as ready for review September 20, 2025 05:55
@pinkenburg
Copy link
Contributor

pinkenburg commented Sep 20, 2025 •
edited
Loading

PHParameters are very expensive to use on an event by event basis. They are meant to be used for configurations and have a DB backend. This is just the wrong class to handle event by event parameters in this case,

@Steepspace
Copy link
Contributor Author

Steepspace commented Sep 20, 2025

PHParameters are very expensive to use on an event by event basis. They are meant to be used for configurations and have a DB backend. This is just the wrong class to handle event by event parameters in this case,

I will note that PHParameters is currently used in the JetBackgroundCut module that also does something similar on an event by event basis:

coresoftware/offline/packages/jetbackground/JetBackgroundCut.cc

Lines 283 to 293 in 9d03281

PHCompositeNode *parNode = dynamic_cast<PHCompositeNode *>(iter.findFirst("PHCompositeNode", "PAR"));
_cutParams.set_int_param("failsLoEmJetCut", failsLoEm);
_cutParams.set_int_param("failsHiEmJetCut", failsHiEm);
_cutParams.set_int_param("failsIhJetCut", failsIhCut);
_cutParams.set_int_param("failsAnyJetCut", failsAnyCut);
_cutParams.set_int_param("isDijet", isDijet);
_cutParams.set_double_param("frcem", frcem);
_cutParams.set_double_param("frcoh", frcoh);
_cutParams.set_double_param("maxJetET", maxJetET);
_cutParams.set_double_param("dPhi", dPhi);
_cutParams.UpdateNodeTree(parNode, "JetCutParams");

However, since this is not recommended, do you suggest any other approach that will be ideal to pass the information regarding the cuts so that QA downstream can log that different cases?

@pinkenburg
Copy link
Contributor

pinkenburg commented Sep 20, 2025

That was a bit of a temporary measure - before that they used PHFlag which is even worse. The very basic problem here (and in the jet background) is that it uses the temporary "PAR" which is not saved (and also not reset - so there is a possibility of carrying information from one event to the next accidentally). That means the downstream qa module can never be run off a DST which is just a bad idea (and it will potentially read info from a previous event). I don't see any qa code which accesses this node - so I am not sure if this was actually thought out or followed up with an actual qa implementation.
What we probably need is some dumb i/o class with an int and double vector and then have accessors for each implementation which just look values up via index and store this thing under the DST node
.

@Steepspace
Copy link
Contributor Author

Steepspace commented Sep 20, 2025

What we probably need is some dumb i/o class with an int and double vector and then have accessors for each implementation which just look values up via index and store this thing under the DST node

Thank you for the suggestion. Could you provide a template or reference that I can use to implement this i/o class that can store these quantities under the DST node?
In the meantime, please feel free to mark this PR back to Draft since it's not acceptable at this stage.

@pinkenburg pinkenburg marked this pull request as draft September 20, 2025 23:23
@sphenix-jenkins-ci
Copy link

sphenix-jenkins-ci bot commented Oct 31, 2025

Build & test report

Report for commit ef1c109a81e6aeebdf52f3a02261fea1c7da033b:
Jenkins on fire

Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@Steepspace
Update MinimumBiasClassifier - Add PHParamters
9e7adf5 
- No functional changes introduced
- Simply propagating the the failure cases for the isAuAuMiniumBias flag.
@Steepspace
Update MinimumBiasClassifier - Suppress MB Check
b21ea65 
Suppressing the minimum bias check setting to false for total MBD charge above 2100.
@Steepspace
Update MinimumBiasClassifier - Adjust Defaults Loc
9405084 
- Setting the defaults for the MinBias params at the top of the FillMinimumBiasInfo method to ensure they are set
@Steepspace
Propagate the MBD Information
949ddba 
- MBD Total Charge (North & South), vertex scale and centrality scale factor
@Steepspace
MinimumBiasClassifier - MBD Charge Thresh Var
8ce0e14 
- Converted the total MBD Charge threshold from hard coded to into variable and swtiched to the new default of 2700
@Steepspace
MinimumBiasClassifier - Default MBD Charge Max
8c1ecbe 
- Adjust default MBD Charge Max back to it's default of 2100
@Steepspace
Minimum Bias Params Refactor
6aa5d2a 
- Move the creation of minimum bias params outside of the GetNodes which
is called once per each event
- Only needs to be called once in the init
@sphenix-jenkins-ci
Copy link

sphenix-jenkins-ci bot commented Nov 14, 2025

Build & test report

Report for commit 6aa5d2a73f9ba02afcc703a4bca98ae19d5351e5:
Jenkins on fire

Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Steepspace @pinkenburg