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

chi2_vs_drj and pull_vs_pt validation plots #31400

Conversation

vberta
Copy link
Contributor

@vberta vberta commented Sep 9, 2020

PR description:

This PR include some additional validation tracking plots:

  • chi2 vs dR(track,jet): reco,associated,prob
  • pull vs pT: 2D distribution, mean, sigma
  • the deltaR(jet,track) upper limit has been moved to dRj=0.5

In addition, this PR setup the MTV to be able to easily switch to associator by chi2 (commented lines with proper documentation).

These new features are useful to monitor jetCore iteration and high-pT kinematic region more in general. Another PR will follow in next releases, with a relevant fix in Cluster Repair (on Edge Hits) integration. The plots of this PR are needed to check the improvement of the fix.

PR validation:

This PR has been validated on small number of events of QCD_Pt_1800to2400_TuneCP5_13TeV_pythia8 sample, with the following path:

  1. cmsDriver.py step2 --conditions auto:phase1_2018_realistic -s DIGI:pdigi_valid,L1,DIGI2RAW,HLT:@relval2018 --datatier GEN-SIM-DIGI-RAW -n 20000 --geometry DB:Extended --era Run2_2018 --eventcontent FEVTDEBUGHLT --filein file:step1.root --fileout file:step2.root --no_exec

  2. cmsDriver.py step3 --conditions auto:phase1_2018_realistic -n 20000 --era Run2_2018 --eventcontent RECOSIM,MINIAODSIM,DQM --runUnscheduled -s RAW2DIGI,L1Reco,RECO,RECOSIM,EI,PAT,VALIDATION:@standardValidation+@miniAODValidation,DQM:@standardDQM+@ExtraHLT+@miniAODDQM --datatier GEN-SIM-RECO,MINIAODSIM,DQMIO --geometry DB:Extended --filein file:step2.root --fileout file:step3.root --no_exec

  3. cmsDriver.py step4 --conditions auto:phase1_2018_realistic -s HARVESTING:@standardValidation+@standardDQM+@ExtraHLT+@miniAODValidation+@miniAODDQM --scenario pp --filetype DQM --geometry DB:Extended --era Run2_2018 --mc -n 20000 --filein file:step3_inDQM.root --fileout file:step4.root --no_exec

A validation on 20k events is currently ongoing.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2020

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2020

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31400/18249

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2020

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2020

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31400/18250

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2020

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

It involves the following packages:

SimTracker/TrackAssociation
SimTracker/TrackAssociatorProducers
Validation/RecoTrack

@andrius-k, @kmaeshima, @civanch, @mdhildreth, @cmsbuild, @jfernan2, @fioriNTU can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @abbiendi, @GiacomoSguazzoni, @JanFSchulte, @jhgoh, @VinInn, @rovere, @wmtford, @mtosi, @ebrondol, @threus, @dgulhan 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

@mmusich
Copy link
Contributor

mmusich commented Sep 9, 2020

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2020

The tests are being triggered in jenkins.

@mmusich
Copy link
Contributor

mmusich commented Sep 9, 2020

urgent

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2020

+1
Tested at: 5bed2bc
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4e4e18/9211/summary.html
CMSSW: CMSSW_11_2_X_2020-09-08-2300
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2020

Comparison job queued.

@mtosi
Copy link
Contributor

mtosi commented Sep 9, 2020

could this PR be approved, please ?

@jfernan2
Copy link
Contributor

jfernan2 commented Sep 9, 2020

I'm referring to all workflows which either runs FakeHLT or not run the HLT at all

So, when is Tracking DQM expecting to perform such cleaning?

@mtosi
Copy link
Contributor

mtosi commented Sep 9, 2020

this has nothing to do with the tracking DQM,
but it should be related to the HLT validation workflow, right ?
or better, to the definition of the workflow to be run in runTheMatrix
which should be under the responsibility of PdmV (and maybe DQM) ?

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 9, 2020

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4e4e18/9232/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 35
  • DQMHistoTests: Total histograms compared: 2609667
  • DQMHistoTests: Total failures: 4935
  • DQMHistoTests: Total nulls: 9778
  • DQMHistoTests: Total successes: 2594932
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 71561.987 KiB( 34 files compared)
  • DQMHistoSizes: changed ( 10024.0,... ): 2273.004 KiB Tracking/Track
  • DQMHistoSizes: changed ( 10024.0,... ): 1430.106 KiB Tracking/TrackBuilding
  • DQMHistoSizes: changed ( 10024.0,... ): 671.399 KiB HLT/Tracking
  • DQMHistoSizes: changed ( 10024.0,... ): 384.744 KiB HLT/Muon
  • DQMHistoSizes: changed ( 10024.0,... ): 383.602 KiB Tracking/TrackFromPV
  • DQMHistoSizes: changed ( 10024.0,... ): 95.804 KiB HLT/EGM
  • DQMHistoSizes: changed ( 10024.0,... ): 87.500 KiB Tracking/TrackTPPtLess09
  • DQMHistoSizes: changed ( 10024.0,... ): 43.359 KiB Tracking/TrackConversion
  • DQMHistoSizes: changed ( 10024.0,... ): 37.244 KiB Tracking/TrackFromPVAllTP
  • DQMHistoSizes: changed ( 10024.0,... ): 21.694 KiB Tracking/TrackAllTPEffic
  • DQMHistoSizes: changed ( 10024.0 ): ...
  • Checked 149 log files, 22 edm output root files, 35 DQM output files

@civanch
Copy link
Contributor

civanch commented Sep 10, 2020

+1

@jfernan2
Copy link
Contributor

this has nothing to do with the tracking DQM,
but it should be related to the HLT validation workflow, right ?
or better, to the definition of the workflow to be run in runTheMatrix
which should be under the responsibility of PdmV (and maybe DQM) ?

I don't fully agree: Tracking sequences are on the HLT validation paths, they were introduced at some point, you see it now with this PR, once you increase the number of Tracking MEs, it gets propagated.

@mtosi
Copy link
Contributor

mtosi commented Sep 10, 2020

this discussion I think does not have anything to do w/ this PR, which was supposed to be urgent
and therefore I'm really sorry for this situation

said that, I see that for instance the workflow 10024.0 has this issue w/ the HLT validation

I have just run
runTheMatrix.py -l 10024.0 --what upgrade --dryRun
and I find
cmsDriver.py step2 --conditions auto:phase1_2017_realistic -s DIGI:pdigi_valid,L1,DIGI2RAW,HLT:@relval2017 --datatier GEN-SIM-DIGI-RAW -n 10 --geometry DB:Extended --era Run2_2017 --eventcontent FEVTDEBUGHLT --filein file:step1.root --fileout file:step2.root > step2_TTbar_13+2017+TTbar_13TeV_TuneCUETP8M1_GenSim+Digi+Reco+HARVEST+ALCA+Nano.log 2>&1

and looking for HLT:@relval2017, I find the config
https://cmssdt.cern.ch/dxr/CMSSW/source/Configuration/HLT/python/autoHLT.py#11
which, indeed, it seems to call a fake menu
=> the HLT validation should not be run at all on this workflow

while the workflow runs
cmsDriver.py step3 --conditions auto:phase1_2017_realistic -s RAW2DIGI,L1Reco,RECO,RECOSIM,EI,PAT,VALIDATION:@standardValidation+@miniAODValidation,DQM:@standardDQM+@ExtraHLT+@miniAODDQM --datatier GEN-SIM-RECO,MINIAODSIM,DQMIO -n 10 --geometry DB:Extended --era Run2_2017 --eventcontent RECOSIM,MINIAODSIM,DQM --filein file:step2.root --fileout file:step3.root > step3_TTbar_13+2017+TTbar_13TeV_TuneCUETP8M1_GenSim+Digi+Reco+HARVEST+ALCA+Nano.log 2>&1

=> the VALIDATION step pretends to run the whole HLT validation sequence
VALIDATION:@standardValidation+@miniAODValidation,DQM:@standardDQM+@ExtraHLT+@miniAODDQM
on a fake menu, which is instead running only
https://cmssdt.cern.ch/dxr/CMSSW/source/HLTrigger/Configuration/python/HLT_Fake2_cff.py#203

as said, this does not have anything do to w/ the tracking DQM/Validation
one should update the workflow to use
https://cmssdt.cern.ch/dxr/CMSSW/source/Validation/Configuration/python/autoValidation.py#17
instead of
https://cmssdt.cern.ch/dxr/CMSSW/source/Validation/Configuration/python/autoValidation.py#16
I guess

@mtosi
Copy link
Contributor

mtosi commented Sep 10, 2020

@silviodonato could this PR still be integrated in 11_2_0_pre6 ?
thanks

@jfernan2
Copy link
Contributor

jfernan2 commented Sep 10, 2020

Using FakeHLT is not the way to go since phase2 workflows are the ones to test new HLT menus until data taking.
The point is that any addition/change you do to _trackingParticleRefSelector
is automatically propagated to all the HLT modules doing their own Tracking validation which uses that exactly module through trackValidation_cff, i.e.
https://github.com/cms-sw/cmssw/search?q=TrackValidation_cff&unscoped_q=TrackValidation_cff

Nevertheless I agree this has nothing to do with this PR itself, but it appears everytime TrackValidation is touched. resulting in 1k new MEs being added in one single shot

@jfernan2
Copy link
Contributor

+1

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

@mmusich
Copy link
Contributor

mmusich commented Sep 10, 2020

Using FakeHLT is not the way to go since phase2 workflows are the ones to test new HLT menus until data taking.

You mean Run 3 workflows .. right?

@silviodonato
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 715db98 into cms-sw:master Sep 10, 2020
@jfernan2
Copy link
Contributor

Using FakeHLT is not the way to go since phase2 workflows are the ones to test new HLT menus until data taking.

You mean Run 3 workflows .. right?

Yes, sorry my typo...

@mtosi
Copy link
Contributor

mtosi commented Sep 10, 2020

@silviodonato @jfernan2 thanks

I would suggest opening a CMSSW issue for handling the problem stressed by @jfernan2
where all the involved parties (DQM, PdmV and probably STEAM) can contribute
in parallel the TRK POG/DQM could add some flags in order to handle part of the plots which are maybe not that relevant in specific phase-space, but let me stress that these new plots have been added because they were missing ...

@mmusich
Copy link
Contributor

mmusich commented Sep 10, 2020

@jfernan2

The point is that any addition/change you do to _trackingParticleRefSelector
is automatically propagated to all the HLT modules doing their own Tracking validation which uses that exactly module through trackValidation_cff, i.e.
https://github.com/cms-sw/cmssw/search?q=TrackValidation_cff&unscoped_q=TrackValidation_cff

Mia can chime in, but I think that's done (for the time being) on purpose. I can agree that possibly not all the plots are necessary for all clones, though the problem of having a real HLT validation sequence being run for a workflow which does not run the HLT, is a true issue, IMHO.
Perhaps it's time for another round of cleaning as done here #28156 ?

@jfernan2
Copy link
Contributor

I do think so @mmusich even if it has not been a year since such cleaning was performed...
Thanks

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

7 participants