search

cmstas / NtupleMaker

produces CMS3 Ntuples. See NtupleTools for auxiliary functions
1 stars 10 forks source link

pfIso in ElectronMaker #7

Closed jgran closed 10 years ago

jgran commented 10 years ago

https://github.com/cmstas/NtupleMaker/blob/7_0_0_dev/src/ElectronMaker.cc https://github.com/cmstas/NtupleMaker/blob/7_0_0_dev/python/electronMaker_cfi.py

gzevi commented 10 years ago

The lines that cause the issue are (in RecoConfiguration2012_cfg.py): from CommonTools.ParticleFlow.Tools.pfIsolation import setupPFElectronIso, setupPFPhotonIso process.eleIsoSequence = setupPFElectronIso(process, 'gedGsfElectrons')

The issue is: ValueError: Trying to override definition of pfSelectedPhotons while it is used by the sequence pfElectronTranslatorSequence new object defined in: /afs/cern.ch/cms/slc5_amd64_gcc481/cms/cmssw/CMSSW_7_0_0_pre12/python/CommonTools/ParticleFlow/ParticleSelectors/pfSelectedPhotons_cfi.py existing object defined in: /afs/cern.ch/cms/slc5_amd64_gcc481/cms/cmssw/CMSSW_7_0_0_pre12/python/RecoParticleFlow/PFProducer/pfBasedElectronPhotonIso_cff.py

These lines were used to recalculate electron isolation on the fly due to a bug in the AOD variable. This was done following the instructions on: https://twiki.cern.ch/twiki/bin/viewauth/CMS/EgammaPFBasedIsolation

This is no longer needed because the AOD variables are correct starting with release 6. A lot of the relevant CMSSW sequences have changed, so it is not surprising that the recipe designed for Rel 5 doesn't work in Rel 7. I cannot find a way to fix this issue without modifying the CMSSW sequences directly, which seems risky. Maybe we can leave this aside and only come back to it if the EGamma POG finds a new isolation bug and suggests a new recipe?

gzevi commented 10 years ago

I spoke with EGamma POG people (Daniele Benedetti) and learned the following.

  1. As we had noticed above, the old recommendation for electron isolation no longer works, so we shouldn't use it.
  2. On the other hand there is no recommended version or tool yet, and the isolation variables in the AOD are wrong. So we should just sit tight and wait for a tool or recipe to produce release-7 isolation.
  3. The new tool or recipe will most likely rely on a new MAP used to associate non-PF electrons to whatever PF objects they were reconstructed as. In other words, if a GSF electron is reconstructed as a PF pion, this map will link the GSF electron and the PF pion so that we can remove the PF pion from the isolation cone.

Bottom line: I think I will try to add this map ("particleBasedIsolation_gedGsfElectrons") to CMS2, then hopefully close this issue.

claudiocc1 commented 10 years ago

huh?? Isolation in the AOD is wrong in release 7? Really? Slava: what do you know about this? How can it be wrong?

On 2/12/14 8:19 AM, gzevi wrote:

I spoke with EGamma POG people (Daniele Benedetti) and learned the following.

  1. As we had noticed above, the old recommendation for electron isolation no longer works, so we shouldn't use it.
  2. On the other hand there is no recommended version or tool yet, and the isolation variables in the AOD are wrong. So we should just sit tight and wait for a tool or recipe to produce release-7 isolation.
  3. The new tool or recipe will most likely rely on a new MAP used to associate non-PF electrons to whatever PF objects they were reconstructed as. In other words, if a GSF electron is reconstructed as a PF pion, this map will link the GSF electron and the PF pion so that we can remove the PF pion from the isolation cone.

Bottom line: I think I will try to add this map ("particleBasedIsolation_gedGsfElectrons") to CMS2, then hopefully close this issue.

— Reply to this email directly or view it on GitHub https://github.com/cmstas/NtupleMaker/issues/7#issuecomment-34885012.

IHateLinus commented 10 years ago

I suspect “wrong” is used as not completely consistent between GSFele and the iso ingredients…

On Feb 12, 2014, at 8:25 AM, Claudio Campagnari notifications@github.com wrote:

huh?? Isolation in the AOD is wrong in release 7? Really? Slava: what do you know about this? How can it be wrong?

On 2/12/14 8:19 AM, gzevi wrote:

I spoke with EGamma POG people (Daniele Benedetti) and learned the following.

  1. As we had noticed above, the old recommendation for electron isolation no longer works, so we shouldn't use it.
  2. On the other hand there is no recommended version or tool yet, and the isolation variables in the AOD are wrong. So we should just sit tight and wait for a tool or recipe to produce release-7 isolation.
  3. The new tool or recipe will most likely rely on a new MAP used to associate non-PF electrons to whatever PF objects they were reconstructed as. In other words, if a GSF electron is reconstructed as a PF pion, this map will link the GSF electron and the PF pion so that we can remove the PF pion from the isolation cone.

Bottom line: I think I will try to add this map ("particleBasedIsolation_gedGsfElectrons") to CMS2, then hopefully close this issue.

— Reply to this email directly or view it on GitHub https://github.com/cmstas/NtupleMaker/issues/7#issuecomment-34885012.

  • UCSB: CERN: *
  • Office: Broida 5119 Office: B510 1-011 or 1-015 *
  • Phone +1 805 893 7567 Phone: +41 22 76 71748 or 79496 *
  • Mobile: +41 (0) 762727404 *

    — Reply to this email directly or view it on GitHub.

claudiocc1 commented 10 years ago

who is "IHateLinus"?

On 2/12/14 8:28 AM, IHateLinus wrote:

I suspect “wrong” is used as not completely consistent between GSFele and the iso ingredients…

On Feb 12, 2014, at 8:25 AM, Claudio Campagnari notifications@github.com wrote:

huh?? Isolation in the AOD is wrong in release 7? Really? Slava: what do you know about this? How can it be wrong?

On 2/12/14 8:19 AM, gzevi wrote:

I spoke with EGamma POG people (Daniele Benedetti) and learned the following.

  1. As we had noticed above, the old recommendation for electron isolation no longer works, so we shouldn't use it.
  2. On the other hand there is no recommended version or tool yet, and the isolation variables in the AOD are wrong. So we should just sit tight and wait for a tool or recipe to produce release-7 isolation.
  3. The new tool or recipe will most likely rely on a new MAP used to associate non-PF electrons to whatever PF objects they were reconstructed as. In other words, if a GSF electron is reconstructed as a PF pion, this map will link the GSF electron and the PF pion so that we can remove the PF pion from the isolation cone.

Bottom line: I think I will try to add this map ("particleBasedIsolation_gedGsfElectrons") to CMS2, then hopefully close this issue.

— Reply to this email directly or view it on GitHub

https://github.com/cmstas/NtupleMaker/issues/7#issuecomment-34885012.

  • UCSB: CERN: *
  • Office: Broida 5119 Office: B510 1-011 or 1-015 *
  • Phone +1 805 893 7567 Phone: +41 22 76 71748 or 79496 *
  • Mobile: +41 (0) 762727404 *

    — Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub https://github.com/cmstas/NtupleMaker/issues/7#issuecomment-34886180.

jgran commented 10 years ago

It looks like Slava isn't a member of the cmstas organization, so he won't see these comments. You will need to ping him separately or someone can add him.

IHateLinus commented 10 years ago

Slava is on the SnT mailing list though…

On Feb 12, 2014, at 8:50 AM, Jason Gran notifications@github.com wrote:

It looks like Slava isn't a member of the cmstas organization, so he won't see these comments. You will need to ping him separately or someone can add him.

— Reply to this email directly or view it on GitHub.

IHateLinus commented 10 years ago

me

On Feb 12, 2014, at 8:29 AM, Claudio Campagnari notifications@github.com wrote:

who is "IHateLinus"?

On 2/12/14 8:28 AM, IHateLinus wrote:

I suspect “wrong” is used as not completely consistent between GSFele and the iso ingredients…

On Feb 12, 2014, at 8:25 AM, Claudio Campagnari notifications@github.com wrote:

huh?? Isolation in the AOD is wrong in release 7? Really? Slava: what do you know about this? How can it be wrong?

On 2/12/14 8:19 AM, gzevi wrote:

I spoke with EGamma POG people (Daniele Benedetti) and learned the following.

  1. As we had noticed above, the old recommendation for electron isolation no longer works, so we shouldn't use it.
  2. On the other hand there is no recommended version or tool yet, and the isolation variables in the AOD are wrong. So we should just sit tight and wait for a tool or recipe to produce release-7 isolation.
  3. The new tool or recipe will most likely rely on a new MAP used to associate non-PF electrons to whatever PF objects they were reconstructed as. In other words, if a GSF electron is reconstructed as a PF pion, this map will link the GSF electron and the PF pion so that we can remove the PF pion from the isolation cone.

Bottom line: I think I will try to add this map ("particleBasedIsolation_gedGsfElectrons") to CMS2, then hopefully close this issue.

— Reply to this email directly or view it on GitHub

https://github.com/cmstas/NtupleMaker/issues/7#issuecomment-34885012.

  • UCSB: CERN: *
  • Office: Broida 5119 Office: B510 1-011 or 1-015 *
  • Phone +1 805 893 7567 Phone: +41 22 76 71748 or 79496 *
  • Mobile: +41 (0) 762727404 *

    — Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub https://github.com/cmstas/NtupleMaker/issues/7#issuecomment-34886180.

  • UCSB: CERN: *
  • Office: Broida 5119 Office: B510 1-011 or 1-015 *
  • Phone +1 805 893 7567 Phone: +41 22 76 71748 or 79496 *
  • Mobile: +41 (0) 762727404 *

    — Reply to this email directly or view it on GitHub.

IHateLinus commented 10 years ago

this is not helpful, I guess. iHateLinus = me = avi

On Feb 12, 2014, at 9:06 AM, Avi Yagil ayagil@physics.ucsd.edu wrote:

me

On Feb 12, 2014, at 8:29 AM, Claudio Campagnari notifications@github.com wrote:

who is "IHateLinus"?

On 2/12/14 8:28 AM, IHateLinus wrote:

I suspect “wrong” is used as not completely consistent between GSFele and the iso ingredients…

On Feb 12, 2014, at 8:25 AM, Claudio Campagnari notifications@github.com wrote:

huh?? Isolation in the AOD is wrong in release 7? Really? Slava: what do you know about this? How can it be wrong?

On 2/12/14 8:19 AM, gzevi wrote:

I spoke with EGamma POG people (Daniele Benedetti) and learned the following.

  1. As we had noticed above, the old recommendation for electron isolation no longer works, so we shouldn't use it.
  2. On the other hand there is no recommended version or tool yet, and the isolation variables in the AOD are wrong. So we should just sit tight and wait for a tool or recipe to produce release-7 isolation.
  3. The new tool or recipe will most likely rely on a new MAP used to associate non-PF electrons to whatever PF objects they were reconstructed as. In other words, if a GSF electron is reconstructed as a PF pion, this map will link the GSF electron and the PF pion so that we can remove the PF pion from the isolation cone.

Bottom line: I think I will try to add this map ("particleBasedIsolation_gedGsfElectrons") to CMS2, then hopefully close this issue.

— Reply to this email directly or view it on GitHub

https://github.com/cmstas/NtupleMaker/issues/7#issuecomment-34885012.

  • UCSB: CERN: *
  • Office: Broida 5119 Office: B510 1-011 or 1-015 *
  • Phone +1 805 893 7567 Phone: +41 22 76 71748 or 79496 *
  • Mobile: +41 (0) 762727404 *

    — Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub https://github.com/cmstas/NtupleMaker/issues/7#issuecomment-34886180.

  • UCSB: CERN: *
  • Office: Broida 5119 Office: B510 1-011 or 1-015 *
  • Phone +1 805 893 7567 Phone: +41 22 76 71748 or 79496 *
  • Mobile: +41 (0) 762727404 *

    — Reply to this email directly or view it on GitHub.

olivito commented 10 years ago

I added Slava to the cmstas organization, so now he'll be pestered by these mails.

@Slava, the question is the status of PF iso for electrons in rel 7 and what's saved in the AODs.

On Wed, Feb 12, 2014 at 6:10 PM, IHateLinus notifications@github.comwrote:

this is not helpful, I guess. iHateLinus = me = avi

On Feb 12, 2014, at 9:06 AM, Avi Yagil ayagil@physics.ucsd.edu wrote:

me

On Feb 12, 2014, at 8:29 AM, Claudio Campagnari < notifications@github.com> wrote:

who is "IHateLinus"?

On 2/12/14 8:28 AM, IHateLinus wrote:

I suspect "wrong" is used as not completely consistent between GSFele and the iso ingredients...

On Feb 12, 2014, at 8:25 AM, Claudio Campagnari notifications@github.com wrote:

huh?? Isolation in the AOD is wrong in release 7? Really? Slava: what do you know about this? How can it be wrong?

On 2/12/14 8:19 AM, gzevi wrote:

I spoke with EGamma POG people (Daniele Benedetti) and learned the following.

  1. As we had noticed above, the old recommendation for electron isolation no longer works, so we shouldn't use it.
  2. On the other hand there is no recommended version or tool yet, and the isolation variables in the AOD are wrong. So we should just sit tight and wait for a tool or recipe to produce release-7 isolation.
  3. The new tool or recipe will most likely rely on a new MAP used to associate non-PF electrons to whatever PF objects they were reconstructed as. In other words, if a GSF electron is reconstructed as a PF pion, this map will link the GSF electron and the PF pion so that we can remove the PF pion from the isolation cone.

Bottom line: I think I will try to add this map ("particleBasedIsolation_gedGsfElectrons") to CMS2, then hopefully close this issue.

Reply to this email directly or view it on GitHub

https://github.com/cmstas/NtupleMaker/issues/7#issuecomment-34885012.

  • UCSB: CERN: *
  • Office: Broida 5119 Office: B510 1-011 or 1-015 *
  • Phone +1 805 893 7567 Phone: +41 22 76 71748 or 79496 *
  • Mobile: +41 (0) 762727404 *

    -- Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHub https://github.com/cmstas/NtupleMaker/issues/7#issuecomment-34886180.

  • UCSB: CERN: *
  • Office: Broida 5119 Office: B510 1-011 or 1-015 *
  • Phone +1 805 893 7567 Phone: +41 22 76 71748 or 79496 *
  • Mobile: +41 (0) 762727404 *

    -- Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHubhttps://github.com/cmstas/NtupleMaker/issues/7#issuecomment-34891163 .

olivito commented 10 years ago

whoops, meant @slava77

claudiocc1 commented 10 years ago

Slava,

can you give us a "high level" summary of what is going on.

Last time we had gsfElectrons and pfElectrons. Both collections had very loose cuts, but not 100% overlap.

Now what do we have and what is one supposed to use?

C

On 2/12/14 9:22 AM, Slava Krutelyov wrote:

On 2/12/14, 10:25 AM, Claudio Campagnari wrote:

huh?? Isolation in the AOD is wrong in release 7? Really? Slava: what do you know about this? How can it be wrong?

The fact that isolation in AOD in 70X is wrong, is a news to me (maybe it's a severity of what "wrong" is). Please email to the egamma HN and get a more clear answer.

gedGsfElectrons (tmp version, partially filled) are made before pflow takes all inputs and may decide that the input gedGsfElectron is not a pf electon. After pf algo the gedGsfElectrons are filled with isolation and other pf variables. So, if your gedGsfElectron is a pf electron, the isolation is OK. For all other cases when you don't like pflow's decision and want to still call your gedGsfElectron as an electron, to get a meaningful pf isolation you need to remove the pf candidates that came out of your original gedGsfElectron.

  --slava

On 2/12/14 8:19 AM, gzevi wrote:

I spoke with EGamma POG people (Daniele Benedetti) and learned the following.

  1. As we had noticed above, the old recommendation for electron isolation no longer works, so we shouldn't use it.
  2. On the other hand there is no recommended version or tool yet, and the isolation variables in the AOD are wrong. So we should just sit tight and wait for a tool or recipe to produce release-7 isolation.
  3. The new tool or recipe will most likely rely on a new MAP used to associate non-PF electrons to whatever PF objects they were reconstructed as. In other words, if a GSF electron is reconstructed as a PF pion, this map will link the GSF electron and the PF pion so that we can remove the PF pion from the isolation cone.

Bottom line: I think I will try to add this map ("particleBasedIsolation_gedGsfElectrons") to CMS2, then hopefully close this issue.

— Reply to this email directly or view it on GitHub https://github.com/cmstas/NtupleMaker/issues/7#issuecomment-34885012.

gzevi commented 10 years ago

Let me try while we wait for Slava:

Last time we had gsfElectrons and pfElectrons. The same electron could be found in both collections with different energies, since the energy reconstruction was quite different. This implied two separate energy scales, different shapes of variables, etc.

This time we just have gedGsfElectrons: a single collection that contains all electron candidates, whether they pass the GED selection or not. The energy is calculated uniformly for all electrons, using something called "Refined Supercluster".

I couldn't find a complete reference, but here's a decent one: https://indico.cern.ch/event/283895/material/slides/0?contribId=4

The reason for the "wrong" isolation is the following, as far as I understood: If a gedGsfElectrons does not pass the GED selection, it is classified as some other particle by the PF. When asking for the PF-isolation, this other particle should clearly not be included in the sum (since it is the electron itself), but this subtraction is not done correctly.

slava77 commented 10 years ago

My direct messages from my email client to this issue thread didn't make it.

1) response to the wrong AOD:

The fact that isolation in AOD in 70X is wrong, is a news to me
(maybe it's a severity of what "wrong" is).
Please email to the egamma HN and get a more clear answer.

gedGsfElectrons (tmp version, partially filled) are made before pflow takes
all inputs and may decide that the input gedGsfElectron is not a pf electon.
After pf algo the gedGsfElectrons are filled with isolation and other pf
variables.
So, if your gedGsfElectron  is a pf electron, the isolation is OK.
For all other cases when you don't like pflow's decision and want to
still call your gedGsfElectron as an electron, to get a meaningful pf
isolation you need to remove the pf candidates
that came out of your original gedGsfElectron.

2) response to the "high level" summary

For most use cases it's more or less the same now, only gsfElectrons are
now called gedGsfElectrons.
IIRC, as in the past, the pfElectrons are a subset of the (ged)gsf
electrons.

Now you have  more information about what the gsf electron turned into
in the pf candidate collection.
IHateLinus commented 10 years ago

sounds to me like a tempest in a tea kettle. we should store hat’s in the AOD as well as the MAP (we already have PFcands) and we should be good to go, for now. we can and will revisit and refine, I am sure.. but we can start testing.

On Feb 12, 2014, at 2:31 PM, slava77 notifications@github.com wrote:

My direct messages from my email client to this issue thread didn't make it.

1) response to the wrong AOD:

The fact that isolation in AOD in 70X is wrong, is a news to me (maybe it's a severity of what "wrong" is). Please email to the egamma HN and get a more clear answer.

gedGsfElectrons (tmp version, partially filled) are made before pflow takes all inputs and may decide that the input gedGsfElectron is not a pf electon. After pf algo the gedGsfElectrons are filled with isolation and other pf variables. So, if your gedGsfElectron is a pf electron, the isolation is OK. For all other cases when you don't like pflow's decision and want to still call your gedGsfElectron as an electron, to get a meaningful pf isolation you need to remove the pf candidates that came out of your original gedGsfElectron. 2) response to the "high level" summary

For most use cases it's more or less the same now, only gsfElectrons are now called gedGsfElectrons. IIRC, as in the past, the pfElectrons are a subset of the (ged)gsf electrons.

Now you have more information about what the gsf electron turned into in the pf candidate collection. — Reply to this email directly or view it on GitHub.

gzevi commented 10 years ago

I agree with the tea kettle point. We are working with a pre-release after all.

I have added the map (intss_electronMaker_elspfcandidx_CMS2) to the CMS2 ntuple. I have not added an isolation that uses this map, for the moment, because this is still being tested by the EGamma POG. In any case, with the MAP and the PFCands we should be good to go.

gzevi commented 10 years ago

This is my first time adding a map to CMS2. I tried to do it safely, using the PFJetMaker pfjets_pfcandIndicies as an example, even though it seems slow. If someone could take a look at my commit I would appreciate it. https://github.com/cmstas/NtupleMaker/commit/f3032ad74850ed4171ba4c3884e5e2d833222ab2 Thank you

gzevi commented 10 years ago

Giuseppe showed me how to store the indexes without looping over all the candidates. Code is much cleaner now, and gives the same results. https://github.com/cmstas/NtupleMaker/commit/73f81c769523ce35d88a45071fc6ee206c5ad137