GED EGM RECO as default + customizeOldEGReco #1834
Conversation
|
A new Pull Request was created by @lgray (Lindsey Gray) for CMSSW_7_0_X. GED EGM RECO as default + customizeOldEGReco It involves the following packages: Calibration/EcalAlCaRecoProducers @thspeer, @giamman, @lveldere, @vlimant, @demattia, @danduggan, @franzoni, @rovere, @cmsbuild, @anton-a, @nclopezo, @rcastello, @deguio, @slava77, @eliasron, @fabiocos can you please review it and eventually sign? Thanks. |
|
+1 |
|
This pull request is not mergeable. @lgray (Lindsey Gray) could you please rebase it? Please have a look at: http://cms-sw.github.io/cmssw/tutorial-forward-port.html if you do not know how. |
|
The conflict is the following: |
|
@ktf Fixed now. |
1 similar comment
|
+1 |
|
-1 ----- Begin Fatal Exception 17-Dec-2013 16:03:23 CET-----------------------
An exception of category 'ProductNotFound' occurred while
[0] Processing run: 1 lumi: 1 event: 1
[1] Running path 'local_digireco'
[2] Calling event method for module InterestingDetIdFromSuperClusterProducer/'interestingEcalDetIdPFEB'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: std::vector
Looking for module label: particleFlowSuperClusterECAL
Looking for productInstanceName: particleFlowSuperClusterECALBarrel
Additional Info:
[a] If you wish to continue processing events after a ProductNotFound exception,
add "SkipEvent = cms.untracked.vstring('ProductNotFound')" to the "options" PSet in the configuration.
----- End Fatal Exception -------------------------------------------------
|
|
Working on it... This one is weird. |
|
This is some problem with the validation being configured in a weird way. The rest of the RECO chain works. |
|
I have a combined fixed-up version in |
|
@slava77, let me know if the error in #1834 (comment) is still a problem. I didn't manage to exactly reproduce it on top of this pull at least, but can try again on top of yours. |
|
Thanks for reminding of this , added 101.0 to my tests |
|
Interestingly enough it crashed in a different place [1] Running path 'local_digireco' |
|
Ok, "good", this is also what I see. Will take a look... |
|
additional replace will be necessary to get back |
|
Ok, |
|
This should be fixed by bendavid@b4d26fb, based on top of this commit. |
|
@slava77 working on it |
|
+1 |
|
(continuing on diffs in the oldEG, with a previous version of this PR) pfPhoton plots are also in agreement (no diffs, in fact) gedPhotons are also in a much better agreement gsfElectrons are pretty different (were about the same wrt 1801) I'm guessing the new regression explains the changes |
|
Hi Slava, only the pt changed on the ttbar? thanks, On 19/12/13 14:39, slava77 wrote:
|
|
@dbenedet Ah, good point. We cannot switch back to the converted brem find, so the fake rate will be higher for non-isolated guys. Sorry, I thought we could perfectly make a No-op, this is not true. We've also identified another change which makes the customizeForOldEGReco not completely a no-op: |
|
(in case this didn't get through .. i sent from my mail client and this message didn't show up .. I wonder why and how many more I missed in other places :-/ ) Hi Daniele, You can see all plots in https://slava77sk.web.cern.ch/slava77sk/reco/relvaldiffs/all_sign288-noGEDVSsign284_TTbarPUwf202p0.tar.gz |
working towards total no-op with respect to old RECO. May not be possible due to new converted brem finder.
|
In the meantime, on the event content in RECO: from 16-0200 IB to that+#1834 in oldEG mode
The same in AOD:
16-0200 IB -> that+#1834 (new default)
The same in AOD
Does this match your needs? |
|
Not sure why PhotonIDProd got dropped... Checking with latest commits. |
Trying to make it work on gedPhotons is wrought with peril and variously tentacled demons. ^^^^^ We should eventually fix this part, especially the demons.
|
I'm running the tests now.
|
|
+1 Final tests done in CMSSW_7_0_X_2013-12-18-1400 (as sign289) in the new default and the oldEG mode. Decision is based on the results above, code diffs in the last iteration, and the summary below. @lgray Lindsey (below) there are quite a few empty plots for conversions in the oldEG mode. I understand that they are either not used for the default photons or can be validated appropriately in the other (oldpf and pf photon validators) Note: I'm comparing to the IB itself shows expected differences introduced by 1801 (and 1862). Comparisons to pre-1801 is not done (can't just diff with my old tests in IB 16-0200, because the Run2 tracking was added in between, which reshuffles everything). New default mode compared to IB (1801 is here) withGED mode
noGED mode: comparing 1834 in oldEG mode vs IB
DQM:
|
|
+1 |
|
+1 |
|
@civanch @mdhildreth @lveldere @rcastello @demattia @diguida, I'll wait a bit more for your approval, but then I'll bypass it since this is the last showstopper for 700pre11 and I want this in the 1400 build. |
|
@slava77 Is the fake rate for electrons (look at low-pT gsf Electrons in TTbar) lower in OldEG mode now? |
|
Lindsey, |
GED EGM RECO as default + customizeOldEGReco
This is built on top of CMSSW_7_0_X_2013-12-18-1400.
You will need to manually download the new MVA weight file, for the classifiers, in the following packages:
RecoParticleFlow/PFTracking
RecoEgamma/ElectronIdentification
with the usual
cat download.url | xargs wgetFor comparison purposes the default running mode of this PR should be a no-op to "GED mode" for #1801.
Likewise "OldEG" mode should be a no-op to the default running mode from before #1801, except for tracker-driven only GSF electrons where we cannot use the old soft-electron MVA or or converted brem finder. This leads to a slightly increased fake rate.
I've updated FastSim and Skims where necessary.
OldEG mode is activated by doing:
runTheMatrix.py -s --useInput all --command="--customise RecoEgamma/Configuration/customizeOldEGReco.customizeOldEGReco"@slava77 The default mode only runs one version of the of conversion tracking. Please confirm timing improvements. The OldEG mode runs both sets of conversion tracking now.
@nancymarinelli In default mode, standard reco::Photons no longer have conversion information.
@franzoni All validation sequences (that I could find) have been updated to point at the new ged products. If there's anything deeper you need please let us know.
Matrix tests on my side: