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 provenance of electrons #2890

Merged
merged 3 commits into from Apr 4, 2014
Merged

Conversation

beaudett
Copy link
Contributor

Hello

as requested by Lindsey & Daniele, fixed the transmission of the provenance from the PFlow to the GsfElectron (variable MvaOutput.status) The change has been implemented in the producer previously called "PFIsolationFiller" which has been renamed into GEDGsfElectronFinalizer.
For the time being, the provenance is recomputed by checking the presence of the PFCandidate of type electron with the same GsfTrack. In the future, more information could be provided through the status flag of the PFEgammaExtra. When it is done, the present code will just keep it and no longer recompute it.

F.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @beaudett (Florian Beaudette) for CMSSW_7_1_X.

Fix provenance of electrons

It involves the following packages:

RecoEgamma/EgammaElectronProducers

@nclopezo, @cmsbuild, @anton-a, @thspeer, @slava77, @Degano can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@cmsbuild
Copy link
Contributor

@anton-a
Copy link

anton-a commented Mar 19, 2014

checking...

@anton-a
Copy link

anton-a commented Mar 20, 2014

I was doing the test mostly to check the timing, but observed some small differences. They have little effect, but I would like to understand the mechanism. Obviously the classification has changed. Probably the rest follows from there. THe plots below are from ttbar+PU.

all_pr2890_vs_orig_ttbarpuwf202p0c_recogsfelectrons_gedgsfelectrons__reco_obj_classification

all_pr2890_vs_orig_ttbarpuwf202p0c_recogsfelectrons_gedgsfelectrons__reco_obj_px

@beaudett
Copy link
Contributor Author

Hello,

the change in the classification is indeed expected to change a little the momentum of the electrons... but what I don't understand is that the change in the classification is in an other PR.
#2872

F.

@anton-a
Copy link

anton-a commented Mar 20, 2014

Yes... that is exactly what I thought. Is it essential for this PR that #2872 is already applied?

@beaudett
Copy link
Contributor Author

Hello Anton,

no, the two PR are independent.
Hope it helps,
Florian

@anton-a
Copy link

anton-a commented Mar 20, 2014

Then it is strange...
I can try merging against an IB that already has 2872.

@beaudett
Copy link
Contributor Author

Hello Anton,
I checked and it is seems that because I am still far from mastering git, I included in this branch the fix of the provenance. That's explains the differences you are observing.
I hope it is not going to make the merging difficult.

Cheers,
Florian

@slava77
Copy link
Contributor

slava77 commented Apr 4, 2014

+1

#2890 55d98cf
tested in CMSSW_7_1_X_2014-04-02-0200 (test area sign335).
No differences observed in monitored quantities
The diff reported earlier has apparently gone away after the fix of the provenance went to the IB

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 4, 2014

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

ktf added a commit that referenced this pull request Apr 4, 2014
Reco -- Fix provenance of electrons
@ktf ktf merged commit c2ee106 into cms-sw:CMSSW_7_1_X Apr 4, 2014
@beaudett beaudett deleted the fix-provenance branch July 21, 2017 13:45
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

6 participants