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

Training files for DNN-based Tau-Ids #1

Merged
merged 12 commits into from Dec 5, 2018
Merged

Conversation

mbluj
Copy link
Contributor

@mbluj mbluj commented Oct 26, 2018

As title says: this PR adds initial versions of training files for DNN-based Tau-Ids. Related to cms-sw/cmssw#25016

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mbluj for branch master.

@cmsbuild, @smuzaffar, @gudrutis, @mrodozov 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.

external issue cms-sw/cmsdist#4455

@slava77
Copy link

slava77 commented Oct 26, 2018

@mbluj
the size is pretty large.
Is it the model optimized/minimized for inference or is it the training model (which is usually much larger and contains extra data not needed for inference).

@mbluj
Copy link
Contributor Author

mbluj commented Oct 26, 2018

@slava77, do you refer to a particular training file (the biggest one?) or all of them? Regardless, this question is beyond my knowledge, so I redirect it to authors of the discriminants: @kandrosov, @ocolegrove could you comment please?

@kandrosov
Copy link
Contributor

@mbluj
the size is pretty large.
Is it the model optimized/minimized for inference or is it the training model (which is usually much larger and contains extra data not needed for inference).

@slava77 I can answer about DeepTauId/deepTau_2017v1_20L1024N.pb (the biggest file): yes, this is a model, which is optimized for inference.

@slava77
Copy link

slava77 commented Oct 26, 2018

@slava77, do you refer to a particular training file (the biggest one?) or all of them? Regardless, this question is beyond my knowledge, so I redirect it to authors of the discriminants: @kandrosov, @ocolegrove could you comment please?

I had in mind all of them (there are just 3 with somewhat similar size).

@ocolegrove
Copy link

ocolegrove commented Oct 26, 2018 via email

@mbluj
Copy link
Contributor Author

mbluj commented Oct 26, 2018 via email

@ocolegrove
Copy link

ocolegrove commented Oct 26, 2018 via email

@slava77
Copy link

slava77 commented Oct 26, 2018

I will investigate if the model size could be reduced

please get in touch with BTV developers @riga @mverzett @pablodecm may be able to check quickly.

@riga
Copy link

riga commented Oct 26, 2018

Hi @ocolegrove ,
in case you want to convert your model to a constant graph which reduces the size quite a lot, I summarized the procedure here: https://gitlab.cern.ch/mrieger/CMSSW-DNN#constant-graphs

@ocolegrove
Copy link

ocolegrove commented Oct 26, 2018 via email

@cmsbuild
Copy link
Contributor

Pull request #1 was updated.

external issue cms-sw/cmsdist#4455

@mbluj
Copy link
Contributor Author

mbluj commented Nov 20, 2018

The most recent update replaces original training files by their quantized versions with size reduced by a factor of ~4.

@fabiocos
Copy link

fabiocos commented Dec 3, 2018

@mbluj @slava77 @perrotta I would say we should merge this PR and the corresponding cmsdist one

@perrotta
Copy link

perrotta commented Dec 3, 2018

@fabiocos These are the training files needed by #25016, and therefore they must be made available to it.

@fabiocos
Copy link

fabiocos commented Dec 5, 2018

+externals

@fabiocos
Copy link

fabiocos commented Dec 5, 2018

merge

@cmsbuild cmsbuild merged commit 10eaa13 into cms-data:master Dec 5, 2018
@smuzaffar
Copy link
Contributor

cmsdist PR cms-sw/cmsdist#4554 includes this data package.

@mbluj
Copy link
Contributor Author

mbluj commented Dec 5, 2018

Hello,
maybe it is obvious, but it will be good to have these files also in new 102X (and 94X?) releases where the DNN tau-Ids are backported. Is it considered?

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

10 participants