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

restore class definition using PF information #2872

Merged
merged 1 commit into from Mar 17, 2014

Conversation

beaudett
Copy link
Contributor

Restore the so-called "Bad-track" class which disappeared during the migration to GED. The electrons which end up in this category were previously in the "showering class".
The plots below have been made with RelValZEE 710pre3 samples.
The migration from one class to the other can be seen here:
h_ele_classes
(red is new, blue is default 710pre3; the entries <10 are in the barrel; >=10 in the endcaps)

Even though the regression is using the class and was trained before the fix, it seems that it won't be necessary to retrain it.
h_ele_poptrue_showering_barrel
h_ele_poptrue_showering_endcaps
The resolution is actually slightly improved. The statistics have decreased between the two cases, which is normal.

There are no validation plots for the "badTrack" class, but the inclusive plots is essentially unchanged.
h_ele_poptrue

@cmsbuild
Copy link
Contributor

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

restore class definition using PF information

It involves the following packages:

RecoEgamma/EgammaElectronAlgos

@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

@lgray
Copy link
Contributor

lgray commented Mar 15, 2014

@anton-a Just got your message on skype. This has indeed been coordinated with me, we asked @beaudett to do it. :-)

@anton-a
Copy link

anton-a commented Mar 15, 2014

Thanks - I'll run some single particle MC...

{
ele->setSuperClusterFbrem(0) ;
ele->setPfSuperClusterFbrem(0);
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please clarify: this method is now marked as obsolete but you still make changes and keep it in the code...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm referring to

  • void addPflowInfo() ;
  • void addPflowInfo() ; // now deprecated

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,
as far as I understand, we still want to be able to run the electron reconstruction code in the "oldEgamma" mode, as done in 700pre11 (or pre12). Removing this method will break it this possibility. I added a comment to keep track that we want to drop it eventually.
F.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, for now we should keep it around so long as we want the option to run
in OldEG mode.

-L

On Sat, Mar 15, 2014 at 10:48 PM, Florian Beaudette <
notifications@github.com> wrote:

In RecoEgamma/EgammaElectronAlgos/src/GsfElectronAlgo.cc:

 else
  • { ele->setSuperClusterFbrem(0) ; }
    
  •  {
    
  • ele->setSuperClusterFbrem(0) ;
  • ele->setPfSuperClusterFbrem(0);
  •  }
    
    }

Hello,
as far as I understand, we still want to be able to run the electron
reconstruction code in the "oldEgamma" mode, as done in 700pre11 (or
pre12). Removing this method will break it this possibility. I added a
comment to keep track that we want to drop it eventually.
F.

Reply to this email directly or view it on GitHubhttps://github.com//pull/2872/files#r10636835
.

@anton-a
Copy link

anton-a commented Mar 16, 2014

Tested on the mix of single particles and TTbar, QCD workflows and observed very few bin-to-bin migrations (apart from the electron category change). Probably a result of the small difference due to training before the fix.

@anton-a
Copy link

anton-a commented Mar 16, 2014

+1

@cmsbuild
Copy link
Contributor

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?

nclopezo added a commit that referenced this pull request Mar 17, 2014
RecoEgamma/EgammaElectronAlgos -- restore class definition using PF information
@nclopezo nclopezo merged commit d3d90b2 into cms-sw:CMSSW_7_1_X Mar 17, 2014
@beaudett beaudett deleted the class-fix 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

5 participants