Adding Xi-compatible tracks to lostTracks collection #33891

AdrianoDee · 2021-05-29T10:47:11Z

PR description:

This PR adds to lostTracks collection all those tracks (pions) that can potentially form Λπ candidate checking the invariant mass M( Λπ)< X GeV. This PR adds to the lostTracks modules a variable xiSelection to turn on this selection and a tunable mass cut (xiMassCut ).

PR validation:

All the tests run.

The size increase is <~ 1% evaluated on JetHT Run2018D data with =37

See slide 10 here

- add to lostTracks tracks consistent with coming from a Xi candidate decaying to LambdaPi - adding a flag to turn on the slection

cmsbuild · 2021-05-29T10:52:04Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33891/22943

This PR adds an extra 24KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File PhysicsTools/PatAlgos/python/slimming/lostTracks_cfi.py modified in PR(s): Unify vertex association between CHS and PUPPI #33885

cmsbuild · 2021-05-29T10:52:25Z

A new Pull Request was created by @AdrianoDee for master.

It involves the following packages:

PhysicsTools/PatAlgos

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @gouskos, @jdolen, @JyothsnaKomaragiri, @ahinzmann, @schoef, @emilbols, @swozniewski, @jdamgov, @mbluj, @nhanvtran, @gkasieczka, @clelange, @hatakeyamak, @ferencek, @gpetruc, @andrzejnovak, @mariadalfonso, @seemasharmafnal, @mmarionncern this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

slava77

please provide a timing measurement, preferably using igprof. The nested loop over tracks may be quite costly and then will need to be rewritten to minimize recomputations.

slava77 · 2021-05-29T18:00:42Z

PhysicsTools/PatAlgos/plugins/PATLostTracks.cc

+    }
+    if (xiSelection_) {
+      // selecting potential Xi- -> Lambda pi candidates
+      TLorentzVector p4Lambda;


please switch to a math:: Lorentz vector. Given the cost of computing eta and phi for tracks, I think that math::XYZTLorentzVector is more appropriate.

slava77 · 2021-05-29T18:07:51Z

@cmsbuild please test

cmsbuild · 2021-05-31T05:49:55Z

-1

Failed Tests: UnitTests RelVals RelVals-INPUT AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4cc4e9/15421/summary.html
COMMIT: 40e810e
CMSSW: CMSSW_12_0_X_2021-05-28-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33891/15421/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test runtestPhysicsToolsPatAlgos had ERRORS

RelVals

4.534.53_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT/step2_RunPhoton2012B+RunPhoton2012B+HLTD+RECODR1reHLT+HARVESTDR1reHLT.log
4.224.22_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC/step2_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC.log
7.37.3_CosmicsSPLoose_UP18+CosmicsSPLoose_UP18+DIGICOS_UP18+RECOCOS_UP18+ALCACOS_UP18+HARVESTCOS_UP18/step1_CosmicsSPLoose_UP18+CosmicsSPLoose_UP18+DIGICOS_UP18+RECOCOS_UP18+ALCACOS_UP18+HARVESTCOS_UP18.log

Expand to see more relval errors ...

RelVals-INPUT

4.224.22_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC/step2_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC.log
4.264.26_ZMuSkim2011A+ZMuSkim2011A+HLTDSKIM+RECODR1reHLT+HARVESTDR1reHLT/step2_ZMuSkim2011A+ZMuSkim2011A+HLTDSKIM+RECODR1reHLT+HARVESTDR1reHLT.log
4.274.27_ZElSkim2011A+ZElSkim2011A+HLTDSKIM+RECODR1reHLT+HARVESTDR1reHLT/step2_ZElSkim2011A+ZElSkim2011A+HLTDSKIM+RECODR1reHLT+HARVESTDR1reHLT.log

Expand to see more relval errors ...

AddOn Tests

fastsimcmsDriver.py TTbar_8TeV_TuneCUETP8M1_cfi --conditions auto:run1_mc --fast -n 100 --eventcontent AODSIM,DQM --relval 100000,1000 -s GEN,SIM,RECOBEFMIX,DIGI:pdigi_valid,L1,DIGI2RAW,L1Reco,RECO,EI,VALIDATION --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --datatier GEN-SIM-DIGI-RECO,DQMIO --beamspot Realistic8TeVCollision : FAILED - time: date Sat May 29 20:36:08 2021-date Sat May 29 20:36:06 2021 s - exit: 256
hlt_data_Fake2cmsDriver.py RelVal -s L1REPACK:Full --data --scenario=pp -n 10 --conditions auto:run2_hlt_Fake2 --relval 9000,50 --datatier "RAW" --eventcontent RAW --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run2_2016 --fileout file:RelVal_Raw_Fake2_DATA.root --filein /store/data/Run2016B/JetHT/RAW/v1/000/272/762/00000/C666CDE2-E013-E611-B15A-02163E011DBE.root : FAILED - time: date Sat May 29 20:36:18 2021-date Sat May 29 20:36:11 2021 s - exit: 256
hlt_mc_Fake2

----- Begin Fatal Exception 29-May-2021 20:36:46 CEST-----------------------
An exception of category 'FileOpenError' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing input source of type PoolSource
   [2] Calling RootInputFileSequence::initTheFile()
   [3] Calling StorageFactory::open()
   [4] Calling File::sysopen()
Exception Message:
Failed to open the file 'RelVal_Raw_Fake2_MC.root'
   Additional Info:
      [a] Input file file:RelVal_Raw_Fake2_MC.root could not be opened.
      [b] open() failed with system error 'No such file or directory' (error code 2)
----- End Fatal Exception -------------------------------------------------

Expand to see more addon errors ...

cmsbuild · 2021-05-31T07:09:36Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33891/22967

This PR adds an extra 24KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File PhysicsTools/PatAlgos/python/slimming/lostTracks_cfi.py modified in PR(s): Unify vertex association between CHS and PUPPI #33885

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33891/22967/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33891/22967/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

AdrianoDee · 2021-05-31T12:30:45Z

please provide a timing measurement, preferably using igprof. The nested loop over tracks may be quite costly and then will need to be rewritten to minimize recomputations.

Sure, about this it's worth to move the

if (trkStatus[trkIndx] == TrkStatus::NOTUSED)

above before the p4 calculations. With this implemented the igprof reports:

original
new

The timing for the module goes from ~150 ms per 100 events to 220 ms per 100 events.

slava77 · 2021-05-31T12:51:32Z

please provide a timing measurement, preferably using igprof. The nested loop over tracks may be quite costly and then will need to be rewritten to minimize recomputations.

Sure, about this it's worth to move the

if (trkStatus[trkIndx] == TrkStatus::NOTUSED)

above before the p4 calculations. With this implemented the igprof reports:
* [original](https://adiflori.web.cern.ch/cgi-bin/igprof-navigator/old_perf/1996)

* [new](https://adiflori.web.cern.ch/cgi-bin/igprof-navigator/new_perf/1534)
The timing for the module goes from ~150 ms per 100 events to 220 ms per 100 events.

the igprof shows that the cost of the loop in question is reasonably small and further optimization is not required.
IIUC, this still requires a change in the location of if (trkStatus[trkIndx] == TrkStatus::NOTUSED) compared to the current state of this PR

AdrianoDee · 2021-05-31T12:57:27Z

IIUC, this still requires a change in the location of if (trkStatus[trkIndx] == TrkStatus::NOTUSED) compared to the current state of this PR

Indeed, updated it with the new setup

cmsbuild · 2021-05-31T12:59:31Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33891/22976

This PR adds an extra 24KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File PhysicsTools/PatAlgos/python/slimming/lostTracks_cfi.py modified in PR(s): Unify vertex association between CHS and PUPPI #33885

cmsbuild · 2021-05-31T12:59:55Z

Pull request #33891 was updated. @perrotta, @jpata, @slava77 can you please check and sign again.

slava77 · 2021-05-31T13:32:11Z

@cmsbuild please test

slava77 · 2021-05-31T15:02:04Z

urgent
in the context of getting this to 106X for bParking mini

cmsbuild · 2021-05-31T16:25:56Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4cc4e9/15460/summary.html
COMMIT: 807d8e0
CMSSW: CMSSW_12_0_X_2021-05-30-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33891/15460/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 353 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2650486
DQMHistoTests: Total failures: 803
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2649660
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 36 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 155 log files, 37 edm output root files, 37 DQM output files
TriggerResults: no differences found

mtosi · 2021-06-02T15:19:49Z

thanks a lot for this very fast review (really appreciated !)
is everything fine, or are there still open items ?

thanks, and have a nice day (and Festa della Repubblica for the Italians ;) )

slava77 · 2021-06-03T00:58:07Z

+reconstruction

for #33891 807d8e0

code changes are in line with the PR description and the follow up review
jenkins tests pass and comparisons with the baseline show differences in quantities related to the lostTracks
- DQM plots show more entries in /lostTracks histograms, as expected
- CPU cost seems reasonable based on igprofs provided earlier (the extra cost does not appear significant in the selection loop, while most of the added cost is in creation of the lost track packed candidates)
  - original
  - new
- storage cost is in agreement with the PR description: in wf 136.88811 the output file is up by 0.26%

the wf 136.88811 shows about 26% increase in the number of lost tracks, mostly at low pt

cmsbuild · 2021-06-03T00:58:32Z

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

qliphy · 2021-06-03T03:20:53Z

+1

AdrianoDee · 2021-06-03T06:42:20Z

Thanks!

AdrianoDee added 3 commits May 28, 2021 16:12

Adding tracks compatible with a Xi to lost tracks

4e8135d

- add to lostTracks tracks consistent with coming from a Xi candidate decaying to LambdaPi - adding a flag to turn on the slection

Code checks and code formatting

28116a9

Lowering cut to 1.5

40e810e

cmsbuild added this to the CMSSW_12_0_X milestone May 29, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels May 29, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels May 29, 2021

slava77 reviewed May 29, 2021

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels May 29, 2021

cmsbuild mentioned this pull request May 29, 2021

Unify vertex association between CHS and PUPPI #33885

Merged

cmsbuild added tests-rejected and removed tests-started labels May 31, 2021

Moving to XYZTLorentz vectors

4f044f8

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved tests-rejected labels May 31, 2021

Header cleanup

93bcbbe

cmsbuild added code-checks-rejected and removed code-checks-pending labels May 31, 2021

cmsbuild added the tests-started label May 31, 2021

cmsbuild added code-checks-pending and removed code-checks-approved labels May 31, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels May 31, 2021

cmsbuild added the urgent label May 31, 2021

slava77 mentioned this pull request May 31, 2021

features for bParking UL rereco and miniAOD in 10_6_X (10_6_26?) #33917

Closed

14 tasks

cmsbuild added tests-approved and removed tests-started labels May 31, 2021

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Jun 3, 2021

cmsbuild added orp-approved and removed orp-pending labels Jun 3, 2021

cmsbuild merged commit 220e673 into cms-sw:master Jun 3, 2021

AdrianoDee mentioned this pull request Jun 3, 2021

[Backport] Adding Xi-tracks to lostTracks to 10_6_X #33962

Merged

Adding Xi-compatible tracks to lostTracks collection #33891

Adding Xi-compatible tracks to lostTracks collection #33891

Uh oh!

Conversation

AdrianoDee commented May 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR description:

PR validation:

Uh oh!

cmsbuild commented May 29, 2021

Uh oh!

cmsbuild commented May 29, 2021

Uh oh!

slava77 left a comment

Choose a reason for hiding this comment

Uh oh!

slava77 May 29, 2021

Choose a reason for hiding this comment

Uh oh!

slava77 commented May 29, 2021

Uh oh!

cmsbuild commented May 31, 2021

Unit Tests

RelVals

RelVals-INPUT

AddOn Tests

Uh oh!

cmsbuild commented May 31, 2021

Uh oh!

AdrianoDee commented May 31, 2021

Uh oh!

slava77 commented May 31, 2021

Uh oh!

AdrianoDee commented May 31, 2021

Uh oh!

cmsbuild commented May 31, 2021

Uh oh!

cmsbuild commented May 31, 2021

Uh oh!

slava77 commented May 31, 2021

Uh oh!

slava77 commented May 31, 2021

Uh oh!

cmsbuild commented May 31, 2021

Comparison Summary

Uh oh!

mtosi commented Jun 2, 2021

Uh oh!

slava77 commented Jun 3, 2021

Uh oh!

cmsbuild commented Jun 3, 2021

Uh oh!

qliphy commented Jun 3, 2021

Uh oh!

AdrianoDee commented Jun 3, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AdrianoDee commented May 29, 2021 •

edited

Loading