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

Soft mva muon id implementation #22419

Merged
merged 7 commits into from Mar 21, 2018

Conversation

abragagn
Copy link
Contributor

@abragagn abragagn commented Mar 2, 2018

First implementation of the Soft MVA Muon ID developed for the bmm4 analysis, as discussed in the last Muon POG workshop. Implementation follow the scheme of the lepton MVA implementation (#20612).

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

A new Pull Request was created by @abragagn (a.bragagnolo) for master.

It involves the following packages:

DataFormats/PatCandidates
PhysicsTools/PatAlgos
RecoMuon/MuonIdentification

@perrotta, @monttj, @cmsbuild, @slava77, @gpetruc, @arizzi can you please review it and eventually sign? Thanks.
@TaiSakuma, @gouskos, @abbiendi, @rappoccio, @echapon, @seemasharmafnal, @mmarionncern, @JyothsnaKomaragiri, @ahinzmann, @acaudron, @jhgoh, @jdolen, @HuguesBrun, @drkovalskyi, @ferencek, @trocino, @rociovilar, @rovere, @jdamgov, @bellan, @nhanvtran, @gkasieczka, @schoef, @battibass, @calderona, @mverzett, @bachtis, @cbernet, @gpetruc, @imarches, @mariadalfonso, @pvmulder, @folguera this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@slava77
Copy link
Contributor

slava77 commented Mar 2, 2018

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/26447/console Started: 2018/03/02 15:44

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

-1

Tested at: 671e67d

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-22419/26447/summary.html

I found follow errors while testing this PR

Failed tests: Build

  • Build:

I found an error when building:

>> Compiling edm plugin /build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_1_X_2018-03-01-2300/src/HeavyFlavorAnalysis/RecoDecay/test/stubs/TestBPHRecoDecay.cc 
Entering library rule at HeavyFlavorAnalysis/Onia2MuMu
>> Compiling LCG dictionary: tmp/slc6_amd64_gcc630/src/MuonAnalysis/MomentumScaleCalibration/src/MuonAnalysisMomentumScaleCalibration/a/MuonAnalysisMomentumScaleCalibration_xr.cc
>> Compiling edm plugin /build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_1_X_2018-03-01-2300/src/HeavyFlavorAnalysis/SpecificDecay/test/stubs/TestBPHSpecificDecay.cc 
>> Compiling edm plugin /build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_1_X_2018-03-01-2300/src/HeavyFlavorAnalysis/SpecificDecay/test/stubs/CheckBPHWriteDecay.cc 
error: class 'pat::Muon' has a different checksum for ClassVersion 24. Increment ClassVersion to 25 and assign it to checksum 574733987
Suggestion: You can run 'scram build updateclassversion' to generate src/DataFormats/PatCandidates/src/classes_def_objects.xml.generated with updated ClassVersion
gmake: *** [tmp/slc6_amd64_gcc630/src/DataFormats/PatCandidates/src/DataFormatsPatCandidates/libDataFormatsPatCandidates.so] Error 1
Leaving library rule at DataFormats/PatCandidates
>> Compiling edm plugin /build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_1_X_2018-03-01-2300/src/HeavyFlavorAnalysis/SpecificDecay/plugins/BPHWriteSpecificDecay.cc 
>> Compiling edm plugin /build/cmsbld/jenkins/workspace/ib-any-integration/CMSSW_10_1_X_2018-03-01-2300/src/HeavyFlavorAnalysis/SpecificDecay/plugins/BPHHistoSpecificDecay.cc 


@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2018

Comparison not run due to Build errors (RelVals and Igprof tests were also skipped)

@slava77
Copy link
Contributor

slava77 commented Mar 2, 2018

@slava77
Copy link
Contributor

slava77 commented Mar 2, 2018

@abragagn
also, please remove the xml file from this PR.
It's already available in the release.

@slava77
Copy link
Contributor

slava77 commented Mar 2, 2018

@abragagn
to remove the training file, please do "git rebase -i" and remove the 5895f2d commit

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Pull request #22419 was updated. @perrotta, @monttj, @cmsbuild, @slava77, @gpetruc, @arizzi can you please check and sign again.

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 12, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/26784/console Started: 2018/03/12 10:42

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 7 differences found in the comparisons
  • DQMHistoTests: Total files compared: 29
  • DQMHistoTests: Total histograms compared: 2477919
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2477742
  • DQMHistoTests: Total skipped: 176
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 1.26000000008 KiB( 23 files compared)
  • Checked 118 log files, 9 edm output root files, 29 DQM output files

@slava77
Copy link
Contributor

slava77 commented Mar 12, 2018

+1

for #22419 9403a01

  • changes are in line with the PR description; this affects contents of pat::Muons (miniAOD)
  • jenkins tests pass and comparisons with the baseline show changes only in miniAOD slimmedMuons plots in the selector flags
  • local tests show expected changes: the new MVA variable is filled. The technical performance is somewhat acceptable: CPU increase is small (under 0.1% of miniAOD timing); the memory increase is 8 MB per thread and it should eventually be addressed with a reduction as proposed in Use GBRForest in MuonMvaEstimator and SoftMuonMvaEstimator #20700.

TTbar sample wf 1325.51 shows a reasonable distribution of softMVA value (black is for this PR)
all_origvssign1005_ttbar13reminiaodwf1325p51c_patmuons_slimmedmuons__pat_obj_softmvavalue

@abragagn
Copy link
Contributor Author

Hi @slava77,
We are willing to carry out the work on the GBRForest implementation to reduce the memory usage, but currently none of us has the time to work on it in a meaningful way. It's ok to do it a bit later?

@slava77
Copy link
Contributor

slava77 commented Mar 12, 2018 via email

@fabiocos
Copy link
Contributor

+1

@fabiocos
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 4e9eadc into cms-sw:master Mar 21, 2018
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