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
Add first hit from HitPattern in PackedCandidates with high level track details #19509
Conversation
A new Pull Request was created by @arizzi for master. It involves the following packages: DataFormats/PatCandidates @perrotta, @cmsbuild, @slava77, @monttj, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
probably need to revert last commit or at least adjust it as we store that info for all tracks... |
Comparison job queued. |
the broken unit test seem unrelated with this PR |
Comparison is ready Comparison Summary:
|
-1 Tested at: 15490d4 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: You can see the results of the tests here: I found follow errors while testing this PR Failed tests: AddOn
I found errors in the following addon tests: cmsDriver.py TTbar_Tauola_13TeV_TuneCUETP8M1_cfi -s GEN,SIM,DIGI,L1,DIGI2RAW --mc --scenario=pp -n 10 --conditions auto:run2_mc_GRun --relval 9000,50 --datatier "GEN-SIM-RAW" --eventcontent RAWSIM --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run2_2017 --magField 38T_PostLS1 --fileout file:RelVal_Raw_GRun_MC.root : FAILED - time: date Mon Jul 10 17:49:02 2017-date Mon Jul 10 17:47:37 2017 s - exit: 21248 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+1
|
I do not understand the errors, seem unrelated to me. |
On 7/11/17 9:26 AM, arizzi wrote:
I do not understand the errors, seem unrelated to me.
the addOn error is unrelated to this PR and is now fixed in IB
…
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#19509 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcbnwPwr5ZiKK2aVsqMF_ag9s926kdks5sM3fpgaJpZM4OL5Vs>.
|
merge |
@@ -716,6 +724,8 @@ namespace pat { | |||
lostInnerHitsMask = 0x30, lostInnerHitsShift=4, | |||
muonFlagsMask = 0x0600, muonFlagsShift=9 | |||
}; | |||
public: | |||
uint16_t firstHit_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like I missed that this was added as a public member in the initial review.
@cms-sw/core-l2 is it safe to change this to private or would the schema evolution/root have problems with bw compatibility?
I think it would be OK. @pcanal what is your thoughts? |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-19509/32128 |
The accessibility of a data member does not affect in any way the I/O. So it is safe to change it to private. |
great, I made a PR #39423 ; we'll see what the bot shows in the test results |
This adds the first hit layer/substructure information for PackedCandidates passing the high level track details (i.e.> 0.95GeV at the moment)
It also removes the storage of case the lostHits = -1 (validHitInFirstPixelBarrelLayer) as the information is redundant (at least for tracks where the first hit is in PXB1, perhaps not true for some loopers... )
The size increase should be ~ 60-100 bytes/event
@gpetruc