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 muon MVA ID #36179

Merged
merged 10 commits into from
Feb 15, 2022
Merged

Adding muon MVA ID #36179

merged 10 commits into from
Feb 15, 2022

Conversation

andrea21z
Copy link
Contributor

PR description:

This PR includes the implementation of a new muon identification approach using a MVA:

PR validation:

  • We check the output of the new branch and Working Points running runTheMatrix and everything looks fine.
  • We execute the basic tests suggested in the CMSSW PR instructions.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36179/26748

  • This PR adds an extra 68KB to repository

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

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36179/26749

  • This PR adds an extra 72KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

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

@jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@rappoccio, @gouskos, @schoef, @emilbols, @CeliaFernandez, @mbluj, @demuller, @seemasharmafnal, @mmarionncern, @JyothsnaKomaragiri, @ahinzmann, @jhgoh, @amagitte, @jdolen, @HuguesBrun, @azotz, @cericeci, @trocino, @sscruz, @abbiendi, @rovere, @jdamgov, @nhanvtran, @gkasieczka, @hatakeyamak, @mariadalfonso, @AlexDeMoor, @battibass, @Fedespring, @calderona, @cbernet, @gpetruc, @andrzejnovak 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 19, 2021

test parameters:

@slava77
Copy link
Contributor

slava77 commented Nov 19, 2021

enable profiling

@slava77
Copy link
Contributor

slava77 commented Nov 19, 2021

@cmsbuild please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-eedaa5/20628/summary.html
COMMIT: 2891ee1
CMSSW: CMSSW_12_2_X_2021-11-19-1100/slc7_amd64_gcc900
Additional Tests: PROFILING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36179/20628/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

----- Begin Fatal Exception 19-Nov-2021 16:55:06 CET-----------------------
An exception of category 'InvalidReference' occurred while
   [0] Processing  Event run: 277069 lumi: 81 event: 36060560 stream: 0
   [1] Running path 'dqmofflineOnPAT_step'
   [2] Prefetching for module SingleTopTChannelLeptonDQM_miniAOD/'singleTopElectronMediumDQM_miniAOD'
   [3] Prefetching for module PATMuonSlimmer/'slimmedMuons'
   [4] Prefetching for module PATMuonSelector/'selectedPatMuons'
   [5] Calling method for module PATMuonProducer/'patMuons'
Exception Message:
BadRefCore RefCore: Request to resolve a null or invalid reference to a product of type 'std::vector<reco::Track>' has been detected.
Please modify the calling code to test validity before dereferencing.
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 19-Nov-2021 16:55:08 CET-----------------------
An exception of category 'InvalidReference' occurred while
   [0] Processing  Event run: 305064 lumi: 36 event: 54800878 stream: 0
   [1] Running path 'dqmofflineOnPAT_step'
   [2] Prefetching for module SingleTopTChannelLeptonDQM_miniAOD/'singleTopElectronMediumDQM_miniAOD'
   [3] Prefetching for module PATMuonSlimmer/'slimmedMuons'
   [4] Prefetching for module PATMuonSelector/'selectedPatMuons'
   [5] Calling method for module PATMuonProducer/'patMuons'
Exception Message:
BadRefCore RefCore: Request to resolve a null or invalid reference to a product of type 'std::vector<reco::Track>' has been detected.
Please modify the calling code to test validity before dereferencing.
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 19-Nov-2021 16:55:12 CET-----------------------
An exception of category 'InvalidReference' occurred while
   [0] Processing  Event run: 320822 lumi: 17 event: 27421422 stream: 0
   [1] Running path 'dqmofflineOnPAT_step'
   [2] Prefetching for module SingleTopTChannelLeptonDQM_miniAOD/'singleTopElectronMediumDQM_miniAOD'
   [3] Prefetching for module PATMuonSlimmer/'slimmedMuons'
   [4] Prefetching for module PATMuonSelector/'selectedPatMuons'
   [5] Calling method for module PATMuonProducer/'patMuons'
Exception Message:
BadRefCore RefCore: Request to resolve a null or invalid reference to a product of type 'std::vector<reco::Track>' has been detected.
Please modify the calling code to test validity before dereferencing.
----- End Fatal Exception -------------------------------------------------
Expand to see more relval errors ...

RelVals-INPUT

  • 1325.5181325.518_TTbar_13_reminiaod2018UL_INPUT+TTbar_13_reminiaod2018UL_INPUT+REMINIAOD_mc2018UL+HARVESTUP18_REMINIAOD_mc2018UL/step2_TTbar_13_reminiaod2018UL_INPUT+TTbar_13_reminiaod2018UL_INPUT+REMINIAOD_mc2018UL+HARVESTUP18_REMINIAOD_mc2018UL.log

@jpata
Copy link
Contributor

jpata commented Nov 22, 2021

Looks like the test failures are relevant - please check for the products you are consuming.

The model is very large, 1000 trees and depth 10, 50MB binary weight file. In case the profiling will report a prohibitive memory or CPU consumption, can you investigate ways to prune the model? Did you check if a simple feedforward DNN was competitive?

@jpata
Copy link
Contributor

jpata commented Feb 15, 2022

thank you!

@jpata
Copy link
Contributor

jpata commented Feb 15, 2022

+reconstruction

@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 Feb 15, 2022

+1

@cmsbuild cmsbuild merged commit e29e696 into cms-sw:master Feb 15, 2022
@makortel
Copy link
Contributor

Did this PR cause the CMSSW_12_3_X_2022-02-15-1100 build failure

>> Checking EDM Class Version for src/DataFormats/PatCandidates/src/classes_def_objects.xml in libDataFormatsPatCandidates.so
/data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/4aafea3d3458adee86f697901fdd32f6/opt/cmssw/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_3_X_2022-02-15-1100/src/FWCore/Utilities/scripts/edmCheckClassVersion -l libDataFormatsPatCandidates.so -x /data/cmsbld/jenkins/workspace/build-any-ib/w/tmp/BUILDROOT/4aafea3d3458adee86f697901fdd32f6/opt/cmssw/slc7_amd64_gcc10/cms/cmssw/CMSSW_12_3_X_2022-02-15-1100/src/DataFormats/PatCandidates/src/classes_def_objects.xml
  error: class 'pat::Muon' has a different checksum for ClassVersion 29. Increment ClassVersion to 30 and assign it to checksum 4221614933
 Suggestion: You can run 'scram build updateclassversion' to generate src/DataFormats/PatCandidates/src/classes_def_objects.xml.generated with updated ClassVersion

https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/slc7_amd64_gcc10/CMSSW_12_3_X_2022-02-15-1100/DataFormats/PatCandidates

(only PR touching any muon DataFormats in that or preceding IB, i.e. since the last full build) At this time it is not clear to me what exactly is going on there.

@makortel
Copy link
Contributor

Hmh, I see in the build log of the PR tests
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-eedaa5/22397/build.log
I see DataFormats/PatCandidates was included (as it should), but edmCheckClassVersion was not run?

@smuzaffar
Copy link
Contributor

It was run

>> Checking EDM Class Version for src/DataFormats/PatCandidates/src/classes_def_objects.xml in libDataFormatsPatCandidates.so
>> Checking EDM Class Version for src/DataFormats/PatCandidates/src/classes_def_trigger.xml in libDataFormatsPatCandidates.so
>> Checking EDM Class Version for src/DataFormats/PatCandidates/src/classes_def_user.xml in libDataFormatsPatCandidates.so
>> Checking EDM Class Version for src/DataFormats/PatCandidates/src/classes_def_other.xml in libDataFormatsPatCandidates.so

but I also see root errors like

Error in <TCling::RegisterModule>: Dictionary trigger function for DataFormatsPatCandidates_x3r not found
Error in <TCling::RegisterModule>: Dictionary trigger function for DataFormatsPatCandidates_x2r not found
Error in <TCling::RegisterModule>: Dictionary trigger function for DataFormatsPatCandidates_x1r not found
Error in <TCling::RegisterModule>: Dictionary trigger function for DataFormatsPatCandidates_xr not found

@makortel
Copy link
Contributor

Thanks @smuzaffar, I was searching the wrong string. The error is actually already in there

>> Checking EDM Class Version for src/DataFormats/PatCandidates/src/classes_def_objects.xml in libDataFormatsPatCandidates.so
...
error: class 'pat::Muon' has a different checksum for ClassVersion 29. Increment ClassVersion to 30 and assign it to checksum 4221614933

but the compilation didn't report a failure.

@makortel
Copy link
Contributor

I made a fix in #36972

@smuzaffar
Copy link
Contributor

ah rigth @makortel error is there but scram did not fail. I am checking why it did not fail for PR tests

@smuzaffar
Copy link
Contributor

The issue here is understood now. The happens when one has more than one classes_def xml files and one of these fails. https://github.com/cms-sw/cmssw-config/pull/89/files should fix this issue. The issue is that bash for loop returns the exit code of last command. So if last edm check here passes then exit status of for loop is zero. Change in cms-sw/cmssw-config#89 makes sure that we exit the loop on first error.

@makortel
Copy link
Contributor

Thanks @smuzaffar!

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.

9 participants