-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
DNN-based Tau-Id discrimians (102X) #25386
DNN-based Tau-Id discrimians (102X) #25386
Conversation
- Defined base class for deep tau discriminators. - Removed weight files from home cms repository. Now using weights from cms-data. - Defined WP for both discriminators. Now all discriminators return the corresponding WP results. - Removed cfi files. Using fillDescriptions instead. - General code review and cleaning.
…ection with the new Tau-Ids
DNN tau IDs for CMSSW_10_2_X
…es quantized - Added a new parameter 'version' on runTauIdMVA, used on DPFIsolation - Changes on DeepTauId to reduce memory consumption
…read and reduce the memory consuption - Creation of class DeepTauCache in DeepTauBase, in which now is created graph and session - Implementation of two new static methods inside the class DeepTauBase: initializeGlobalCache and globalEndJob. The graph and DeepTauCache object are created now inside initializeGlobalCache
TauWPThreshold class parses WP cut string (or value) provided in the python configuration. It is needed because the use of the standard StringObjectFunction class to parse complex expression results in an extensive memory usage (> 100 MB per expression).
- Implementation of global cache to avoid reloading graph for each thread - Creation of two new static methods inside the class DeepTauBase: initializeGlobalCache and globalEndJob. The graph and DeepTauCache object are created now inside initializeGlobalCache. The memory consumption of initializeGlobalCache for the original, quantized and files that are load using memory mapping method are in the memory_usage.pdf file - Implemented configuration to use new training files quantized, and set them as default - Implementation of configuration for load files using memory mapping. In our case there wasn't any improvement, respect at the memory consumption of this method, respect the quantized files, so this is not used, but set for future training files - General code review and cleaning.
…ad of the quantized
hold |
Pull request has been put on hold by @smuzaffar |
@smuzaffar , if I understand it correctly, your request in practice is to rebase these pull requests: correct? |
@smuzaffar The PR #25016 has already been merged. It should include the same big files in history as in this PR. Therefore, merging this PR should not consume more space, since the big files are already part of the history in the central repository. |
@kandrosov, It was mistake on our part that #25016 was merged with those big files in the history and now we get warnings like these [a]. We are going to rebase master branch to clean the history and remove ref to these big files. For this back ports PR I would suggest to rebase ( and re-write the history , see detaile here https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History and edit the commits which add these files) [a]
|
I would suggest to clean the history and remove ref to these files [a]. These were added and later deleted in the PR. ERROR: RecoTauTag/RecoTau/data/DPFIsolation_2017v1.pb ['Added', 'Deleted'] |
Hello,
Is it what you mean? |
@mbluj as this and the 9_4_X are backports with no special review in the middle of the history, unless keeping the long list of commits has a value for you I think it might be simpler to follow the advice https://cms-sw.github.io/faq.html#how-do-i-collapse-multiple-commits-into-one @smuzaffar this is what we were discussing as a possibility to trim commit histories, right? |
@fabiocos @smuzaffar I have no problem with rewriting a full history - it takes quite some time, but I can do it in background. However, I would like to understand if procedure I described is correct one (to not mess things). |
unhold |
This pull request is fully signed and it will be integrated in one of the next CMSSW_10_2_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_10_4_X is complete. This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
@mbluj I can just squash myself the history at merge time, if that is ok for you |
Yes, great! Thank you. |
@smuzaffar if I add "+1" will this confuse the bot, or trigger any further action on his side? Just to update the label status for the record.. |
it should be fine to do +1 now. Bot should only update the labels. |
+1 @smuzaffar ok thanks |
This pull request provides two new DNN-based Tau-Ids, DeepTau and DPFTau, to be produced for pat::Taus with MiniAOD.
It is a backport of #25016 to 102X for analyses based on 2018 data and detailed description can be found therein.