events.action: opened
events.author: isobelojalvo
events.text: In 76X we were missing some necessary tau discriminators and had an issue with a new decay mode in the decay mode finding algorithm. The fix was applied in 76X by re-running the Taus at the miniAOD step. This pull request makes the RECO/AOD sequence the same as what is run in the miniAOD sequence in 76X (i.e. integration of needed discriminators + bugfix) and removes the 're-running' of the taus in miniAOD.
events.title: Integration of 76X Tau re-miniAOD into main 80X Sequence
events.type: issue
events.action: created
events.author: slava77
events.text: @isobelojalvo what is the relationship between this PR and #13042 ?
  It looks like the same code is modified, also, the original branch in #13402 was removed, which I guess means that #13402  should be closed.
  Please clarify.
  Thank you.
events.type: comment
events.action: created
events.author: isobelojalvo
events.text: Hi, The 80xReMiniAOD should be used and the 80xReMiniAODv2 had been created as a dummy for testing purposes. I should have notified AJ of this earlier! Apologies! Could you please work off this pull request?
events.type: comment
events.action: created
events.author: slava77
events.text: @cmsbuild please test
events.type: comment
events.action: created
events.author: isobelojalvo
events.text: Checking the unit tests I see they are run on the following sample:
  
  25-Jan-2016 22:04:29 CET  Successfully opened file root://eoscms.cern.ch//eos/cms/store/relval/CMSSW_7_6_0_pre7/RelValProdTTbar_13/AODSIM/76X_mcRun2_asymptotic_v5-v1/00000/0E9A5DE8-1D71-E511-A205-00261894380D.root
  
  The error we see is that MVA6 is not present. This is certainly true: we did not have MVA6 in 7_6_0 at MVA6. Am I missing something? @slava77 @cvuosalo @andrewj314
events.type: comment
events.action: created
events.author: cvuosalo
events.text: @isobelojalvo: For the RECO product of workflow 134.805_RunMET2015C, I find 64 added tau discriminators and 16 removed, for a net gain of 48 discriminators. I think we want to keep the net addition of discriminators to be less than 10. Could you please remove more obsolete and unneeded products?
  
  I see no change in Mini-AOD event content, and I'm still checking AOD.
events.type: comment
events.action: created
events.author: cvuosalo
events.text: @isobelojalvo: The PR description does not describe the results of "fixing" the decay mode algorithm. There are numerous differences showing up for workflow 134.805_RunMET2015C. Are these differences expected? If so, please add an explanation of the expected differences to the PR description. Here are some examples:
  ![oldmseffeta](https://cloud.githubusercontent.com/assets/5736159/12592702/2f9afae8-c433-11e5-84fe-01193b11b573.png)
  ![dmsetatauvis](https://cloud.githubusercontent.com/assets/5736159/12592720/3dcdd568-c433-11e5-8587-1f24a02ed1db.png)
  ![dmeffvspt](https://cloud.githubusercontent.com/assets/5736159/12592719/3dca8f7a-c433-11e5-88c7-4bf1fbbadb7a.png)
events.type: comment
events.action: created
events.author: isobelojalvo
events.text: @cvuosalo Thanks for the plots. Could you tell me if red is from the current pull request or from the old? (or how can I check this myself?) Thank you!
events.type: comment
events.action: created
events.author: cvuosalo
events.text: @isobelojalvo: Yes, red in the plots is from the PR, and black is from the baseline I used, 800pre5.
events.type: comment
events.action: created
events.author: cvuosalo
events.text: @isobelojalvo: I checked AOD event content also, by using workflow 1000.0_RunMinBias2011A. Again, this PR adds 64 new tau discriminators, while only removing 16. We should be aiming to remove almost as many old, unneeded discriminators as new ones being added.
events.type: comment
events.action: created
events.author: slava77
events.type: comment
events.action: created
events.author: cvuosalo
events.text: @slava77: With this PR, there are 140 tau discriminators in AOD for workflow 1000.0_RunMinBias2011A.
events.type: comment
events.action: created
events.author: cvuosalo
events.text: @isobelojalvo: Concerning the unit test failures, the baseline used by Jenkins, CMSSW_8_0_X_2016-01-25-1100, performs the same unit tests successfully. It would seem this PR somehow adds the dependence on MVA6, as indicated by this error message:
  ```
  Principal::getByToken: Found zero products matching all criteria
  Looking for type: reco::PFTauDiscriminator
  Looking for module label: hpsPFTauDiscriminationByMVA6LooseElectronRejection
  ```
events.type: comment
events.action: created
events.author: isobelojalvo
events.text: Hi @cvuosalo, First: apologies about to go offline in a few minutes and will address other issues in the morning. Second: concerning the MVA6, yes, I see it is missing. However, I think this is due to the pattuplization test running on a 76x AOD sample. Dumping the event content of that file shows MVA6 is not there: This is the correct and expected behavior.
events.type: comment
events.action: created
events.author: slava77
events.type: comment
events.action: created
events.author: isobelojalvo
events.text: @slava77 @cvuosalo 
  Relval sample:
  OK, we will see about the new relval sample. I am actually not used to the workflow for the creation of a new relval. Any guidance would help. I will also check with our tau relval experts. In the mean time, is it allowed to run on a privately produce sample instead? And can this pull request be integrated before the new relval is produced? (I am worrying about the timescale here)
  
  Number of Discriminants:
  We have a preference to do the discriminant clean up in a separate branch so that we can keep track if there is an error in the clean up. I created a new branch[*] in cms-tau-pog and will do a pull request when this branch is edited. It should be fast. 
  
  DQM Plots:
  It is expected that the efficiency will increase and this should be in line with what we see in 76xReminiAOD. In the plots, the low efficiency in the black was apparently due to the 3prong+1pi0 integration. It is removed in the current pull request. We are working towards reintegration in the NewDecayModes and not in the OldDecayModes.
  
  [*]https://github.com/cms-tau-pog/cmssw/tree/80xReMiniAOD-RemoveOldDisc
events.type: comment
events.action: created
events.author: slava77
events.type: comment
events.action: created
events.author: slava77
events.text: About the unit tests, I was thinking that we could generate an AOD file on the fly for the case where we know well that the new AOD format is needed.
  
  @gpetruc I seem to remember we did something to make the tests running PAT on AOD inputs to complete in a similar case. Do you remember what was done?
events.type: comment
events.action: created
events.author: isobelojalvo
events.text: The discriminants for removal are listed in the attached files created by Aruna
  
  A rough count shows approximately 1/3 can certainly be removed. The increase overall is partially due to the addition of a new MVA6 electron discriminator and keeping the MVA5, we can remove MVA5 in the next cycle after analysts have had a chance to fully validate MVA6.
  
  [PatTauID_Discriminators_80X.txt](https://github.com/cms-sw/cmssw/files/106993/PatTauID_Discriminators_80X.txt)
  [RecoTauID_Discriminators_80X.txt](https://github.com/cms-sw/cmssw/files/106992/RecoTauID_Discriminators_80X.txt)
events.type: comment
events.action: created
events.author: cvuosalo
events.text: @isobelojalvo, @slava77: I am editing the list of tau discriminators here: https://twiki.cern.ch/twiki/bin/view/CMS/RecoTauDiscrim
events.type: comment
events.action: created
events.author: isobelojalvo
events.text: As suggested, I removed the new inputs. I will put them back in for pre6. Could this be tested?
  
  Thanks
events.type: comment
events.action: created
events.author: slava77
events.text: @cmsbuild please test
events.type: comment
events.action: created
events.author: isobelojalvo
events.text: @cvuosalo concerning the twiki update, I have pointed Arun to the twiki and he will modify as necessary he has also already committed the cleaned up branch. Depending on the release schedule, it would be nice to have the cleaned up branch added to the next release. Let me know if this is a problem for you.
events.type: comment
events.action: created
events.author: cvuosalo
events.text: +1
  
  For #13060 a20e68d1d9f843be26c134fcb066a63c992ff55f
  
  Adds some new tau discriminators and removes a few obsolete ones. Also, removes the 3prong1pi0 decay mode from the AOD sequence.
  
  Follow-up PRs will delete additional unneeded tau discriminators and will add new input for PAT algos after they become available in the next pre-release, as discussed above.
  
  The code changes are satisfactory. Jenkins [tests](https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13060/10818/summary.html) against baseline CMSSW_8_0_X_2016-01-27-2300 show numerous, insignificant differences related to tau decay modes, plus efficiency increases that are expected, as discussed above. Tests of workflow 25202.0_TTbar_13 with 70 events against baseline CMSSW_8_0_0_pre5 show similar differences as those from the Jenkins test results and the plots shown above. An additional example difference plot from this 25202 test is shown below.
  
  Memory and timing measurements show no significant increase. Two timing measurements show no increase, while a third measurement shows an overall timing increase of 1%. At worst, the new tau discriminators may increase times by 18 ms per event, but we expect a follow-up PR that will delete more obsolete discriminators and reduce this timing increase.
  
  Example of change in decay mode finding frequencies:
  ![dmfindsumpt](https://cloud.githubusercontent.com/assets/5736159/12689919/79e6a4fc-c6a5-11e5-8efe-cda581b6735e.png)
events.type: comment
events.action: created
events.author: cvuosalo
events.text: Additional timing measurements from test of workflow 25202 (without Validation step) against baseline CMSSW_8_0_0_pre5:
  
  ```
  Total time formerly used by deleted discriminators: 2.8 ms/ev
  Total time used by new discriminators: 62.3 ms/ev
  
  New discriminators taking > 3 ms/ev
          hpsPFTauDiscriminationByLooseCombinedIsolationDBSumPtCorr3HitsdR03       4.90777 ms/ev
          hpsPFTauDiscriminationByMediumCombinedIsolationDBSumPtCorr3HitsdR03      4.88347 ms/ev
          hpsPFTauDiscriminationByTightCombinedIsolationDBSumPtCorr3HitsdR03       4.88874 ms/ev
          hpsPFTauDiscriminationByIsolationMVArun2v1PWnewDMwLTraw                  3.37712 ms/ev
          hpsPFTauPUcorrPtSumdR03                                                  4.82939 ms/ev
          hpsPFTauNeutralIsoPtSumWeightdR03                                        3.78463 ms/ev
          hpsPFTauDiscriminationByIsolationMVArun2v1DBdR03oldDMwLTraw              3.28005 ms/ev
          hpsPFTauDiscriminationByIsolationMVArun2v1PWdR03oldDMwLTraw              3.40165 ms/ev
  
  
  Retained discriminator that uses the most time, change from baseline to PR:
          hpsPFTauDiscriminationByMVA5rawElectronRejection         15.8576 ms/ev -> 12.6722 ms/ev
  ```
events.type: comment
events.action: created
events.author: deguio
events.text: +1
events.type: comment
events.action: created
events.author: isobelojalvo
events.text: Do we (tau group) have any action items for this pull request? Can it be integrated?
events.type: comment
events.action: created
events.author: slava77
events.text: @isobelojalvo @davidlange6 
  analysis signature is still pending here.
  There is one more PR (13100) that depends on integration of this one.
events.type: comment
events.action: created
events.author: davidlange6
events.text: Right - its been signed by other groups for only a few hours….
  
  >
events.type: comment
events.action: created
events.author: isobelojalvo
events.text: No worries, just want to keep in mind the release closure cycle and not get 'ding-ed'. - Thanks
events.type: comment
data_with_bots.bot: The tests are being triggered in jenkins.
data_with_bots.meta: This message directly preceded comment from bot *cmsbuild* by author *slava77*:
data_with_bots.text: @cmsbuild please test
data_with_bots.bot: The tests are being triggered in jenkins.
data_with_bots.meta: This message directly preceded comment from bot *cmsbuild* by author *slava77*:
data_with_bots.text: @cmsbuild please test
data_without_bots.meta: This message directly preceded comment from bot *cmsbuild* by author *slava77*:
data_without_bots.text: @cmsbuild please test
data_without_bots.meta: This message directly preceded comment from bot *cmsbuild* by author *slava77*:
data_without_bots.text: @cmsbuild please test