Preparations for future trainings of the MVA tauID #18430
Conversation
…eading out BDT value for new decay modes - Addition of read out for new training options: these include the Gottfried-Jackson angle and remove the leading track chi2, which has no discrimination power
Bug fix and addition of new training options for BDT readout
|
A new Pull Request was created by @roger-wolf (Roger Wolf) for master. It involves the following packages: RecoTauTag/RecoTau @perrotta, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here #13028 |
|
@cmsbuild please test |
|
The tests are being triggered in jenkins. |
|
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
| double mAOne = tau->p4().M(); | ||
| double pAOneMag = tau->p(); | ||
| double thetaGJmax = TMath::ASin( (TMath::Power(mTau,2) - TMath::Power(mAOne,2) ) / ( 2 * mTau * pAOneMag ) ); | ||
| double thetaGJmeasured = TMath::ACos( ( tau->p4().px() * decayDistX + tau->p4().py() * decayDistY + tau->p4().pz() * decayDistZ ) / ( pAOneMag * decayDistMag ) ); |
There was a problem hiding this comment.
please use native c++ methods where available.
std::pow, std::acos, std::asin etc)
There was a problem hiding this comment.
I'm missing what by construction restricts the argument of asin to have an absolute value less or equal 1.0?
It looks like there should be a protection.
There was a problem hiding this comment.
Dear Slava,
the valua of mAOne can only be between 0.8 and 1.5 GeV. This cut is placed on 3-prong taus during the tau reconstruction by HPS. Thus, there is no need for protection.
Cheers,
Alex
There was a problem hiding this comment.
asin( (1.77682**2 - 0.8**2)/2/1.77682/pAOneMag)) ~ asin(0.71/pAOneMag)
Does it mean that tau->p() can not possibly be below 0.71 ?
There was a problem hiding this comment.
For that to happen, the transverse momentum of the tau would have to be below 0.71 GeV. In MiniAOD, only taus with pt greater than 18 GeV are saved.
However, in AOD there is no such cut and the minimum pt of the tracks used to reconstruct the tau is 0.5 GeV, I believe. So there it might be necessary.
I will add a statement requiring tau->p() to be greater than 1 although in typical analyses, this will never not be true since typically taus with pt greater than 20 GeV are required.
| @@ -278,6 +292,30 @@ double PATTauDiscriminationByMVAIsolationRun2::discriminate(const TauRef& tau) c | |||
| mvaInput_[20] = ( tau->hasSecondaryVertex() ) ? 1. : 0.; | |||
| mvaInput_[21] = std::sqrt(decayDistMag); | |||
| mvaInput_[22] = std::min((float)10., tau->flightLengthSig()); | |||
| } else if ( mvaOpt_ == kDBoldDMwLTwGJ || mvaOpt_ == kDBnewDMwLTwGJ ) { | |||
| mvaInput_[0] = std::log(std::max((float)1., (float)tau->pt())); | |||
There was a problem hiding this comment.
1.f literal is shorter than (float)1.
same applies to all values below
| mvaInput_[10] = std::min((float)0.5, ptWeightedDrSignal); | ||
| mvaInput_[11] = std::min((float)0.5, ptWeightedDrIsolation); | ||
| mvaInput_[12] = std::min((float)1., eRatio); | ||
| mvaInput_[13] = TMath::Sign((float)+1., tau->dxy()); |
There was a problem hiding this comment.
std::copysign(1.f, tau->dxy());
| double mTau = 1.77682; | ||
| double mAOne = tau->p4().M(); | ||
| double pAOneMag = tau->p(); | ||
| double thetaGJmax = TMath::ASin( (TMath::Power(mTau,2) - TMath::Power(mAOne,2) ) / ( 2 * mTau * pAOneMag ) ); |
There was a problem hiding this comment.
same comments as in PAT tau apply here.
|
It seems like RecoTauTag/RecoTau/plugins/PFRecoTauDiscriminationByMVAIsolationRun2.cc and PATTauDiscriminationByMVAIsolationRun2.cc are badly in need of a good templated helper method to reduce essentially duplicate implementation in two places. |
|
We actually tried doing this when we first developed the code for taus from PAT. However, it is not trivial at all because many variables are accessed differently in |
… and switch to .f literal where possible
|
On 4/27/17 10:35 PM, anehrkor wrote:
Dear Slava,
this behaviour at -1 is expected (and desired) because the new decay
mode (11) is now added to the computation of the MVA value. Before, it
was not the case and the function would return -1 as default
<https://github.com/cms-sw/cmssw/blob/master/RecoTauTag/RecoTau/plugins/PATTauDiscriminationByMVAIsolationRun2.cc#L296>
if the taus is reconstructed in decay mode 11.
Hi Alex,
Thank you.
This is clear.
…
Best,
Alex
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18430 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AEdcbifXdmKTO5cY4L5qz8bE3V37fOvUks5r0XqNgaJpZM4NDsLn>.
|
|
-1 Tested at: 5be50cb The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: You can see the results of the tests here: I found follow errors while testing this PR Failed tests: RelVals
When I ran the RelVals I found an error in the following worklfows: runTheMatrix-results/140.53_RunHI2011+RunHI2011+RECOHID11+HARVESTDHI/step2_RunHI2011+RunHI2011+RECOHID11+HARVESTDHI.log The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped) |
cmsbuild
added
comparison-notrun
tests-rejected
and removed
comparison-pending
tests-started
labels
|
@cmsbuild please test
|
cmsbuild
added
comparison-pending
tests-pending
and removed
comparison-notrun
tests-rejected
labels
|
The tests are being triggered in jenkins. |
|
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
+1 for #18430 5be50cb
|
|
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar |
cmsbuild
added
fully-signed
reconstruction-approved
and removed
pending-signatures
reconstruction-pending
labels
Dear colleagues,
this is a PR by @anehrkor in preparation of new trainings of the MVA tauID. The new 3prong+1pi0 is added to the readout of the MVA and input variables of the MVA are changed. I'm citing the original comments by Alex below (*). We expect changes in the performance of the MVA tauID that Alex will be able to comment on. Apart from such changes our standard tests are inconspicuous.
Cheers,
Roger
(*)
This PR fixes a bug where the decay mode 11 (3-prong+1Pi0) was not considered when reading the BDT output.
Moreover, new training options are added to the readout for upcoming trainings that include the Gottfried-Jackson angle as a variable, and no longer use the leading track chi2.