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

Fix MultiToken.h with missing header include #24423

Merged
merged 3 commits into from Sep 1, 2018
Merged

Fix MultiToken.h with missing header include #24423

merged 3 commits into from Sep 1, 2018

Conversation

guitargeek
Copy link
Contributor

Follow up to #24312 after failing IBs.

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @guitargeek (Jonas Rembser) for master.

It involves the following packages:

RecoEgamma/EgammaTools

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@jainshilpi, @Sam-Harper, @varuns23, @lgray 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 Aug 31, 2018

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 31, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30167/console Started: 2018/08/31 02:40

@kpedro88
Copy link
Contributor

kpedro88 commented Aug 31, 2018

@guitargeek there was also a problem in another file, I forgot to note it before:

src/RecoEgamma/EgammaTools/interface/Utils.h: In function 'void writeValueMap(edm::Event&, const edm::Handle<HandleType>&, const std::vector<ValueType>&, const string&)':
src/RecoEgamma/EgammaTools/interface/Utils.h:15:41: error: 'ValueMap' is not a member of 'edm'
     auto valMap = std::make_unique<edm::ValueMap<ValueType>>();
                                         ^~~~~~~~
src/RecoEgamma/EgammaTools/interface/Utils.h:15:24: error: parse error in template argument list
     auto valMap = std::make_unique<edm::ValueMap<ValueType>>();
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/RecoEgamma/EgammaTools/interface/Utils.h:15:62: error: expected primary-expression before ')' token
     auto valMap = std::make_unique<edm::ValueMap<ValueType>>();
                                                              ^
src/RecoEgamma/EgammaTools/interface/Utils.h:16:19: error: 'ValueMap' in namespace 'edm' does not name a template type
     typename edm::ValueMap<ValueType>::Filler filler(*valMap);
                   ^~~~~~~~
src/RecoEgamma/EgammaTools/interface/Utils.h:16:27: error: expected unqualified-id before '<' token
     typename edm::ValueMap<ValueType>::Filler filler(*valMap);
                           ^
src/RecoEgamma/EgammaTools/interface/Utils.h:17:5: error: 'filler' was not declared in this scope
     filler.insert(handle, values.begin(), values.end());
     ^~~~~~
src/RecoEgamma/EgammaTools/interface/Utils.h:17:5: note: suggested alternative: 'fileno'
     filler.insert(handle, values.begin(), values.end());
     ^~~~~~
     fileno
gmake: *** [config/SCRAM/GMake/Makefile.chk_headers:44: tmp/slc6_amd64_gcc700/check_header/RecoEgamma/EgammaTools/interface/Utils.h] Error 1

@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-24423/30167/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 32
  • DQMHistoTests: Total histograms compared: 3143879
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3143680
  • DQMHistoTests: Total skipped: 197
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
  • Checked 133 log files, 14 edm output root files, 32 DQM output files

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

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

@perrotta
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 31, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/30171/console Started: 2018/08/31 13:04

@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-24423/30171/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 32
  • DQMHistoTests: Total histograms compared: 3143879
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3143681
  • DQMHistoTests: Total skipped: 197
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
  • Checked 133 log files, 14 edm output root files, 32 DQM output files

@slava77
Copy link
Contributor

slava77 commented Aug 31, 2018

+1

for #24423 07ddfa7

  • technical update to keep headers self-consistent (adds some missing includes)
  • jenkins tests pass

@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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@kpedro88
Copy link
Contributor

kpedro88 commented Sep 1, 2018

+1

@cmsbuild cmsbuild merged commit a070467 into cms-sw:master Sep 1, 2018
@guitargeek guitargeek deleted the multitoken_fix branch September 17, 2018 14:31
lucastorterotot pushed a commit to lucastorterotot/cmssw that referenced this pull request Nov 27, 2018
Electron MVA V2 (as in cms-sw#23700 cms-sw#23746 cms-sw#23767)

All event based variables are now handeled by the new helper class

Hardcoding of variables and clips removed for Fall17

Generalized Fall17Iso and Fall17NoIso

Configurable categories and start to move Spring16 MVA

Dedicated variable files for Spring16 and Fall17

Removed Spring16 C++ code

All year specific C++ code removed

Small improvements

Cleared some inconsistencies in variable definitions. All IDs now validated.

Restructured VID python code a bit

New MVAVariableManager helper class

Forgot to actually add the class go git

The raw BDT score is now stored as well

Spring15 PhotonMVA uses now MVAVariableManager

Almost there for photon MVA

All Run2 Photon MVA IDs use now the new PhotonMVAEstimator class

Add variable files

Rewrote some python, ElectronID cuts on raw MWA values and autodetection of gzipped weight files

Implemented V2 MV Ele ID

Cut values are now found with parser as well

Include ntupizer for training

cleaned up a bit

Training Ntuplizer finished

Use auto generatetd config files

Finalized ntuplizer

Auto updated Training ntuplizer

Updated training ntuplizer

Fix in training ntuplizer

fixed wp90 iso V1 parameters

Fix

Added MVA output Validation ntuplizer for photons

Bugfix

Improved exception handling in GBRForestToors

Changed egammaObjectModificationsInMiniAOD_cff.py to comply with new config files

Add two lines in miniAOD_tools.py to cope with ElectronMVAVariableHelper

Fixed Fall17V1 variables

Addressed comments following visual code inspection

Simplified code

Included Fall17 MVAs in MiniAOD and NanoAOD

Added electronMVAVariableHelper to relevant processes

Revisited casting of candidates to electrons/photons

Fixup - prefer dynamic_cast if possible

Corrected the second C style cast in the photons

Avoid undefined category for photons

Changed category not defined error to warning

Changed mvaValue if category not found to -999

Fixed possible memory leak

Drop MVAObjectCache in MVAValueMapProducer

Commit after some code review

Comment out something so branch compiles

Added weightfiles

Some code cleaning (as in ID part of cms-sw#23743)

Adapt GBRForestTools exception handling to 94X

Fall17 cutbased Electron ID V2 (as in cms-sw#23477)

Added Photon MVA weight files

Technical Egamma MVA improvements (as in cms-sw#24131)

Fall17_94X_v2 MVA Photon ID

Fixing mvaClassName in egammaObjectModificationsInMiniAOD_cff.py

Inclusion of V2 IDs in MiniAOD

fix worst charged iso variable

adding photon MVA V2 to the pat::Photon

Fixed copy-paste typo in Spring15 weight files

New MultiTokens (as in cms-sw#24312 cms-sw#24423)

cache AOD/miniAOD values to speed up repeated computations

Egamma IDs back to global cache (as in cms-sw#25101)

Cutbased photonID Fall17 V2

update phoID v2 cutbased

Renamed cut based ID and put in MiniAOD

Modified PhotonNtuplizer to have genMatch information

Added variables from Fall17 ID to PhotonMVANtuplizer

New MVAVariableHelper for extra MVA variables

Moved Photon MVA to new interface

adapted constructors to run MVA estimator in python fwlite

ElectronMVA can be used in FWlite

Egamma Python wrapper class for Electron MVAs

Got rid of ElectronMVAVariableHelper

Adapted PhotonMVANtuplizer

Combined Electron Ntuplizer config files

Fixing problems detected while testing

exposing category in FWLite to be able to apply working point later on

infrastructure to take working points in python+fwlite. implemented for noiso v2 only so far

working points for the other mvas. somthing fishy with GP v1

Spring16 WPs with logistic transform
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

5 participants