-
Notifications
You must be signed in to change notification settings - Fork 14
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
Final BDT models based on 10.2 MC samples #13
Conversation
@mverzett is interested to follow this thread. |
A new Pull Request was created by @bainbrid for branch master. @cmsbuild, @smuzaffar, @gudrutis, @mrodozov can you please review it and eventually sign? Thanks. external issue cms-sw/cmsdist#4702 |
assign reconstruction |
@slava77 ping |
@perrotta will probably have a final say on this. @bainbrid |
Only the three Autumn18 files will be used. The .xml files define the models used as defined by the library used to generate them (scikit-learn), before their conversion to be compliant with TMVA. They are intended to be kept for reproducibility in the future. Do you prefer we remove all unnecessary files from the PR? |
On 2/19/19 12:04 PM, bainbrid wrote:
Do you prefer we remove all unnecessary files from the PR?
yes, please.
Preferably, please rewrite the commits so that the pkl files do not show
up in the git history at all.
|
Pull request #13 was updated. external issue cms-sw/cmsdist#4702 |
@slava77 |
@mrodozov @smuzaffar : could you please prepare two cmsdata pull requests out of this one, one for 10_5_X and one for 10_6_X? Thank you! |
The 10_5_X release is using a different training currently, and the files are named differently.
Doing as you suggested would require configuration changes. I would hold off on this for now, so we can discuss further, perhaps in tomorrow's meeting?
rob
… On 21 Feb 2019, at 17:19, perrotta ***@***.***> wrote:
@mrodozov <https://github.com/mrodozov> @smuzaffar <https://github.com/smuzaffar> : could you please prepare two cmsdata pull requests out of this one, one for 10_5_X and one for 10_6_X?
Thank you!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#13 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABEfksDCYpoJrC1e6ujsSRxGjMCgsWCCks5vPtU3gaJpZM4a8FkL>.
|
So 10_2 only it is for now if I got that right ? |
Apparently yes. Thank you!
Mircho Rodozov <notifications@github.com> ha scritto:
… So 10_2 only it is for now if I got that right ?
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#13 (comment)
|
Description: cms-data/RecoEgamma-ElectronIdentification#13 Resolves: #4702 Needed for: cms-sw/cmssw#25936
After discussing internally, and in the RECO meeting on Friday, we think it would be a good idea to propagate these models to master. Please could you please prepare a new cmsdata pull request out of this one, to be used in master. Ideally, we’d back port to 10_5_X as well, if everyone agrees? I will need to create a new corresponding PR for cms-sw to update the configs to point to the new models (which have different names) and set the appropriate working points. I’ll do this later this evening. |
@mrodozov @smuzaffar could you please prepare the corresponding of #4724 for the master, so that the tests for #26012 can get started? |
@gudrutis , can you please take care of this? Just update the cmsdist/data/cmsswdata.txt with newer version |
I updated RecoEgamma-ElectronIdentification to V01-01-03 on master and started the tests on #26012 . |
@slava77 @perrotta
These BDT models are used by the
LowPtGsfElectronSeedProducer
andLowPtGsfElectronIDProducer
modules in the low pT electron reconstruction.They are to be used in conjunction with by this PR: #25936.
They are intended for the 10_2_X release cycle only. (We will release update models for the 10_5_X release cycles and later.)
The performance of the two Seed models is shown below and compared to the default EGamma seeding approach (blue circle).