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

Adding Disappearing Tracks Cuts (EXO-19-010) to List of saved DeDxHitInfo Cuts in Isolated Tracks Slimming #36225

Merged
merged 7 commits into from Dec 15, 2021
Merged

Conversation

carriganm95
Copy link
Contributor

@carriganm95 carriganm95 commented Nov 23, 2021

PR description:

Added exo disappearing track cuts to slimming/isolatedTracks_cfi.py for use in saveDeDxHitInfoCut. The disappearing tracks cuts are:

  1. pt > 30GeV
  2. abs(dxy) < 0.05 && abs(dz) < 1.0
  3. numMissingInnerHits == 0
  4. numMissingMiddleHits == 0
  5. numMissingOuterHits >=1
  6. pfIsolationDR03().chargedHadronIso + pfIsolationDR03().neutralHadronIso + pfIsolationDR03().photonIso + pfIsolationDR03().puChargedHadronIso)/pt < 0.1

Also added several functions to IsolatedTrack.h that will return the number of missing inner, middle, outer hits to simplify above cuts.

This PR is a part of a previous PR #31399 #31399

PR validation:

Tested by running job processing miniAODSIM from AODSIM data using
root://cmsxrootd.fnal.gov//store/mc/Run3Summer21DR/DYJets_incl_MLL-50_TuneCP5_14TeV-madgraphMLM-pythia8/AODSIM/FlatPU0to70_120X_mcRun3_2021_realistic_v6-v1/230000/000bd8f7-9f34-4619-b0f3-90af8df53617.root

in CMSSW_12_0_3_patch2.

Also followed recommended test procedure but the files required to run "scram b runtests" failed due to the root files not being available.

Checked file size by running over 1000 events on DYJets mcRun 3 sample
root://cmsxrootd.fnal.gov//store/mc/Run3Summer21DR/DYJets_incl_MLL-50_TuneCP5_14TeV-madgraphMLM-pythia8/AODSIM/FlatPU0to70_120X_mcRun3_2021_realistic_v6-v1/230000/000bd8f7-9f34-4619-b0f3-90af8df53617.root

Using CMSSW_12_0_3_patch2 (used to create the dataset being used). Comparing the most recent update to this PR (post rebase) vs CMSSW_12_0_3_patch2 master branch the files are both 66mb and the differences in collections are


master new, B delta, B delta, % deltaJ, % branch

 91.0 ->       126.0         35     32.2   0.05     recoDeDxHitInfos_isolatedTracks__PAT.
596.7 ->       598.2          2      0.3   0.00     patCompositeCandidates_oniaPhotonCandidates_conversions_PAT.
210.4 ->       208.3         -2     -1.0  -0.00   recoGenParticlesedmAssociation_packedPFCandidateToGenAssociation__PAT.
474.2 ->       475.8          2      0.3   0.00     recoCaloJets_slimmedCaloJets__PAT.
 10.4 ->        10.1         -0     -2.5  -0.00     double_fixedGridRhoFastjetAllCalo__RECO. 
3883.5 ->      3880.6         -3     -0.1  -0.00     patTriggerObjectStandAlones_slimmedPatTrigger__PAT.
456.0 ->       451.5         -5     -1.0  -0.01     patJets_slimmedJetsAK8__PAT.  
3876.8 ->      3873.7         -3     -0.1  -0.00     patJets_slimmedJets__PAT.  
3515.8 ->      3513.5         -2     -0.1  -0.00     patMuons_slimmedMuons__PAT. 
633.0 ->       617.7        -15     -2.5  -0.02     patTaus_slimmedTaus__PAT.  
463.1 ->       464.2          1      0.2   0.00     patPhotons_slimmedPhotons__PAT.
987.6 ->       986.1         -1     -0.1  -0.00     recoVertexs_offlineSlimmedPrimaryVerticesWithBS__PAT.
724.4 ->       725.5          1      0.2   0.00     HBHERecHitsSorted_reducedEgamma_reducedHBHEHits_PAT.
892.2 ->       894.1          2      0.2   0.00     patJets_slimmedJetsPuppi__PAT.
  6.1 ->         6.2          0      1.6   0.00     recoBaseTagInfosOwned_slimmedJets_tagInfos_PAT.
1338.2 ->      1341.4          3      0.2   0.00     patElectrons_slimmedLowPtElectrons__PAT.
 10.1 ->        10.3          0      1.9   0.00     recoDeDxHitInfosedmAssociation_isolatedTracks__PAT.
25413.9 ->     25367.4        -46     -0.2  -0.07     patPackedCandidates_packedPFCandidates__PAT.
1102.3 ->      1104.2          2      0.2   0.00     patElectrons_slimmedElectrons__PAT.
-------------------------------------------------------------
66630 ->       66598        -32            -0.0     ALL BRANCHES

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36225/26839

  • This PR adds an extra 28KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • DataFormats/PatCandidates (reconstruction)
  • PhysicsTools/PatAlgos (reconstruction)

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

cms-bot commands are listed here

@slava77
Copy link
Contributor

slava77 commented Nov 25, 2021

@cmsbuild please test

@jpata
Copy link
Contributor

jpata commented Nov 25, 2021

assign @cms-sw/xpog-l2

(for the MINI size check).

@carriganm95 please use a more descriptive PR title.

DataFormats/PatCandidates/interface/IsolatedTrack.h Outdated Show resolved Hide resolved
"numMissingInnerHits == 0 &&"+
"numMissingMiddleHits == 0 &&"+
"numMissingOuterHits >= 1 &&"+
"(pfIsolationDR03().chargedHadronIso + pfIsolationDR03().neutralHadronIso + pfIsolationDR03().photonIso + pfIsolationDR03().puChargedHadronIso)/pt < 0.1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be max(0.,neutralHadronIso+photonIso-0.5*puChargedHadronIso) ?
Considering the length of this string, I suggest to make a helper function in MuonPFIsolation, e.g. combinedWithDeltaBetaCorr or, if the choice in this method with +pu is deliberate combinedWithPU

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we should not have added the pileup. We would like to keep this cut loose so we will simply remove the use of pileup.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we should not have added the pileup. We would like to keep this cut loose so we will simply remove the use of pileup.

is pielup correction really not needed (to subtract from the neutral+photon)? with this definition the reliso<0.1 will be quite inefficient at higher PU.
all other examples in this file are using just the charged iso (which has little pileup dependence)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not do any pileup cleaning in our analysis. The dxy and dz vertex requirements are very tight instead. However, having only chargedHadronIso will not hurt our analysis so we will do that.

Comment on lines 125 to 130
int numMissingInnerHits() const {
return hitPattern_.trackerLayersWithoutMeasurement(reco::HitPattern::MISSING_INNER_HITS);
}
int numMissingMiddleHits() const {
return hitPattern_.trackerLayersWithoutMeasurement(reco::HitPattern::TRACK_HITS);
}
int numMissingOuterHits() const {
return hitPattern_.trackerLayersWithoutMeasurement(reco::HitPattern::MISSING_OUTER_HITS);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are quite confusingly not matching TrackBase::missingInnerHits implementation hitPattern_.numberOfLostHits(HitPattern::MISSING_INNER_HITS)

Looking in other places in cmssw, the shorthand for the trackerLayersWithout variables are lostInnerLayers, lostLayers, and lostOuterLayers, respectively

Suggested change
int numMissingInnerHits() const {
return hitPattern_.trackerLayersWithoutMeasurement(reco::HitPattern::MISSING_INNER_HITS);
}
int numMissingMiddleHits() const {
return hitPattern_.trackerLayersWithoutMeasurement(reco::HitPattern::TRACK_HITS);
}
int numMissingOuterHits() const {
return hitPattern_.trackerLayersWithoutMeasurement(reco::HitPattern::MISSING_OUTER_HITS);
int lostInnerLayers() const {
return hitPattern_.trackerLayersWithoutMeasurement(reco::HitPattern::MISSING_INNER_HITS);
}
int lostLayers() const {
return hitPattern_.trackerLayersWithoutMeasurement(reco::HitPattern::TRACK_HITS);
}
int lostOuterLayers() const {
return hitPattern_.trackerLayersWithoutMeasurement(reco::HitPattern::MISSING_OUTER_HITS);

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-60033e/20769/summary.html
COMMIT: 7cbb238
CMSSW: CMSSW_12_2_X_2021-11-24-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36225/20769/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: 21 differences found in the comparisons
  • DQMHistoTests: Total files compared: 42
  • DQMHistoTests: Total histograms compared: 3247745
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3247723
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 41 files compared)
  • Checked 177 log files, 37 edm output root files, 42 DQM output files
  • TriggerResults: no differences found

@carriganm95 carriganm95 changed the title Dis trk de dx hit info Adding Disappearing Tracks Cuts (EXO-19-010) to List of saved DeDxHitInfo Cuts in Isolated Tracks Slimming Nov 29, 2021
@slava77
Copy link
Contributor

slava77 commented Nov 29, 2021

assign xpog

(for the MINI size check).

@cmsbuild
Copy link
Contributor

New categories assigned: xpog

@mariadalfonso,@gouskos,@fgolf you have been requested to review this Pull request/Issue and eventually sign? Thanks

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36225/26985

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 3, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-60033e/20973/summary.html
COMMIT: d3beb3d
CMSSW: CMSSW_12_2_X_2021-12-03-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36225/20973/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: 24 differences found in the comparisons
  • DQMHistoTests: Total files compared: 41
  • DQMHistoTests: Total histograms compared: 3041955
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3041933
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 40 files compared)
  • Checked 175 log files, 37 edm output root files, 41 DQM output files
  • TriggerResults: no differences found

@slava77
Copy link
Contributor

slava77 commented Dec 7, 2021

the isolated track collection size is

PR: patIsolatedTracks_isolatedTracks__PAT. 1955.95 508.063

master: patIsolatedTracks_isolatedTracks__PAT. 1955.95 507.999

this PR changes the selections for DeDxHitInfo saving. This data is saved in a different branch (not in patIsolatedTracks, it is in recoDeDxHitInfos_isolatedTracks.

Please provide results from compareProducts.sh rootA rootB _ 1 1 ( https://github.com/cms-sw/cms-bot/blob/master/comparisons/compareProducts.sh )

@slava77
Copy link
Contributor

slava77 commented Dec 7, 2021

+reconstruction

for #36225 d3beb3d

  • code changes are in line with the PR description and the follow up review; this affects what goes in the recoDeDxHitInfos_isolatedTracks collection by expanding the range of selections
  • jenkins tests pass and comparisons with the baseline show increase in size of recoDeDxHitInfos_isolatedTracks : data workflows apparently show a larger increase , e.g. in DoubleEG2017C wf 136.793 in 100 events about 60% more tracks have the DeDx data added
    all_mini_OldVSNew_RunDoubleEG2017Cwf136p793c_recoDeDxHitInfos_isolatedTracks__reRECO_obj_infos__charge
    all_mini_OldVSNew_RunDoubleEG2017Cwf136p793c_recoDeDxHitInfos_isolatedTracks__reRECO_obj_size
    in this case the file size increases by 0.1% (52 bytes per event on average).

@carriganm95
Copy link
Contributor Author

Would you like me to repeat this study on the dataset I used? Let me know if there is more to be done.

@slava77
Copy link
Contributor

slava77 commented Dec 8, 2021

Would you like me to repeat this study on the dataset I used? Let me know if there is more to be done.

@carriganm95
please update the PR description with details about the size of recoDeDxHitInfos_isolatedTracks in your test file. The current description is somewhat misleading about the impact on the file size.

@slava77
Copy link
Contributor

slava77 commented Dec 13, 2021

@cms-sw/xpog-l2
please check the event size updates in this PR and perhaps sign if the 0.1% increase is OK.
Thank you.

@perrotta
Copy link
Contributor

@cms-sw/xpog-l2 @mariadalfonso as discussed at the ORP, if the miniAOD size increase is fine with you we can consider this PR for being merged in CMSSW_12_3_0_pre2, supposed to be built before the EOY break.

@gouskos
Copy link
Contributor

gouskos commented Dec 14, 2021

+xpog

@cmsbuild
Copy link
Contributor

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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@qliphy
Copy link
Contributor

qliphy commented Dec 15, 2021

please test
to refresh the test done 11 days ago.

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-60033e/21278/summary.html
COMMIT: d3beb3d
CMSSW: CMSSW_12_3_X_2021-12-14-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36225/21278/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: 24 differences found in the comparisons
  • DQMHistoTests: Total files compared: 42
  • DQMHistoTests: Total histograms compared: 3250719
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3250697
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 41 files compared)
  • Checked 177 log files, 37 edm output root files, 42 DQM output files
  • TriggerResults: no differences found

@perrotta
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit cfc6395 into cms-sw:master Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants