Skip to content
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.

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

(INTT) Updated PHG4InttDigitizer to use CDB calibration #3396

Merged

Conversation

josephbertaux
Copy link
Contributor

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

TODOs (if applicable)

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

@sphenix-jenkins-ci
Copy link

Build & test report

Report for commit 9168e61adb9b231a53495440f50d8db01f169e14:
Jenkins on fire


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@josephbertaux
Copy link
Contributor Author

I want to bring this up at the INTT weekly meeting (tomorrow evening) before fulling green lighting it. Are the Jenkins failures anything I'd need to worry about?

@sphenix-jenkins-ci
Copy link

Build & test report

Report for commit 37b771e51e962fd10133f7c399542920aafade9a:
Jenkins passed


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@pinkenburg
Copy link
Contributor

The intt part of the tracking QA has changed with this PR:
📊 QA-tracking : combined Chi2/nDoF = 79.4999 / 56, and combined p-Value = 0.0211694

@josephbertaux
Copy link
Contributor Author

I'm not sure what to make of these--this is the first I've seen of this type of Jenkins output. It looks like the number of 1-cluster reconstructions for INTT decreased.

Without a hotmap calibration under the sims global tag, the PHG4InttDigitizer won't mask any channels. Before there was probably some arbitrary list. I'm guessing (hoping) this an improvement over what it was before?

Best way to check is to add an identical calibration that corresponds what the digitizer would've masked before the PR, and if nothing changes then we're good. I'll make the CDBTTree by at latest Monday

@josephbertaux
Copy link
Contributor Author

I made a CDBTTree at
/sphenix/user/jbertaux/Repositories/INTT/general_codes/josephb/hotmap_translator/INTT_Hotmap_Sim.root

Could you add the calibration under the sims global tag with payload type "INTT_Hotmap"?

Please do not re-run just yet--I need to make an additional push (change a default argument to actually match this tag), which will trigger Jenkins anyway.

The QA output should be exactly what it was before.

@pinkenburg
Copy link
Contributor

All done - I added the calibration

@josephbertaux
Copy link
Contributor Author

Thank you

@sphenix-jenkins-ci
Copy link

Build & test report

Report for commit 20f63d8a91a30cc3ddf39262375a35e78f6f0df3:
Jenkins on fire


Automatically generated by sPHENIX Jenkins continuous integration
sPHENIX             jenkins.io

@pinkenburg
Copy link
Contributor

clang-tidy is revamped, those errors are not from this PR

@josephbertaux
Copy link
Contributor Author

I see, that last commit was either benign or would break everything b/c I forgot a quote.

Is there a way to look at the simulation reconstruction QA? That that is exactly the same is the only thing I want to verify before this gets merged.

@pinkenburg
Copy link
Contributor

the QA is right here on this page among the jenkins tests, the link gets you to the jupyter notebook with the plots. As long as the p value is 1 there were no changes with respect to the reference

@josephbertaux josephbertaux marked this pull request as ready for review February 8, 2025 21:49
@josephbertaux
Copy link
Contributor Author

Ah, I see it. p=1 and ndof/chi2=0.

Marking "ready for review"

@pinkenburg pinkenburg merged commit 109f93e into sPHENIX-Collaboration:master Feb 8, 2025
25 of 28 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