Fix tau hypo configurations
Trying to remove spurious selections applied after preselection step for tracktwo and track chains. Also removing selections applied after fast tracking (core, iso and isobdt) steps, all of these were not applied during Run 2 (AFAIK)
I think this changes could also fix the phi modulation and other weird ET distributions seen during validation checks ATR-22883. I'm attaching HLT_tau25_mediumRNN_tracktwoMVA Run 2 chain configuration
Merge request reports
Activity
Hi @bernius, I don't know whether these changes will fix the issues that we see in the validation or not, but these are the most likely suspects. In any case we need these changes even if they are not the fix since the configuration of these hypos was wrong. I'm running a large scale test on the grid to see whether the problems are gone or not with these changes. (And this MR is still WIP) Cheers, E
Edited by Edson Carquin Lopez
This merge request affects 3 packages:
- Trigger/TrigHypothesis/TrigTauHypo
- Trigger/TrigValidation/TrigAnalysisTest
- Trigger/TriggerCommon/TriggerMenuMT
Affected files list will not be printed in this case
Adding @sutt ,@martindl ,@ademaria ,@vmartin ,@okumura ,@carquin ,@dzanzi ,@bernius ,@hrussell ,@malconad as watchers
added Tau Trigger TriggerMenu changes-trigger-counts master labels
Very good, thanks for the info @carquin!
-
Athena AthSimulation AthGeneration AnalysisBase AthAnalysis DetCommon externals 
cmake make 
required tests 
optional tests 
Full details available on this CI monitor view
Resolved by Daniele ZanziI don't quite understand the description here. Are you saying that there is a cross talk among chains that is fixed by this MR? have you tested that the cross talk is gone?
- Last reply by Edson Carquin Lopez
added 57 commits
-
712d0a07...76c426e4 - 54 commits from branch
atlas:master - 96b7b1f5 - Trying to fix the hypo configurations
- 28217bc0 - Implement reject empty hypo
- 7cbe694c - Updating references
Toggle commit list-
712d0a07...76c426e4 - 54 commits from branch
added 95 commits
-
7cbe694c...65bfd49d - 92 commits from branch
atlas:master - 11d6dbb3 - Trying to fix the hypo configurations
- 54694d02 - Implement reject empty hypo
- 6861c651 - Fix reference and remove printout
Toggle commit list-
7cbe694c...65bfd49d - 92 commits from branch
-
This merge request affects 4 packages:
- Trigger/TrigHypothesis/TrigTauHypo
- Trigger/TrigValidation/TrigAnalysisTest
- Trigger/TrigValidation/TriggerTest
- Trigger/TriggerCommon/TriggerMenuMT
Affected files list will not be printed in this case
Adding @sutt ,@martindl ,@ademaria ,@vmartin ,@okumura ,@carquin ,@dzanzi ,@bernius ,@hrussell ,@malconad as watchers
-
Athena AthSimulation AthGeneration AnalysisBase AthAnalysis DetCommon externals cmake make required tests optional tests Full details available on this CI monitor view
-
Hello @carquin, overall the MR looks good to me... just let me bring up something we discussed and still don't fully understand.
I agree we should bypass the "TrigTrackPreSelHypo" (the one which is cutting on Iso track and Core Track) for tracktwoEF/tracktwoMVA/tracktwoMVABDT since for these chains we are not applying any preselection step. However, I'm still not getting why we should not apply these cuts on track/tracktwo chains.
Looking at https://cds.cern.ch/record/2274201/files/ATLAS-CONF-2017-061.pdf (page 5), it mentions that these cuts have been applied after the two-stage fast tracking at least until 2017. Also looking at these notes (guess @martindl wrote this) :
Dropping Iso track and Core Track cuts at FTF stage and move the ntrack cut at Precision track level was what brought the transition tracktwo -> tracktwoEF (2018).
So if we want to reproduce the behaviour of track/tracktwo chains in data-taking until 2017, then I think the Iso track and Core Track cuts must be applied.
Edited by Antonio De Maria -
Hello @carquin, as this MR is fixing a bug for the tracktwoEF/tracktwoMVA/tracktwoMVABDT which are for now higher priority chain for validation, please go ahead with this MR. We still cannot claim we solved the phi efficiency modulation for track/tracktwo chains as the changes here are essentially turning off the Preselection step, but we can continue investigating this and place fix in another MR
added 146 commits
-
6861c651...b3a8609a - 143 commits from branch
atlas:master - 052a2607 - Trying to fix the hypo configurations
- effbd1aa - Implement reject empty hypo
- a7064296 - Uodating references
Toggle commit list-
6861c651...b3a8609a - 143 commits from branch
-
Ok, thanks @ademaria I'll unWIP the MR now!
-
great, thanks .... make sure to remove the print messages so review will go smother ;)
Also please do not remove the Hypo properties like
Gaudi::Property m_tracksInCoreCut{ this, "tracksInCoreCut", 3, "" };
Just make sure we by-pass the hypo but leave the code there so we can come back to this later in easier way
Edited by Antonio De Maria added 92 commits
-
a7064296...4bfb7f00 - 88 commits from branch
atlas:master - e200a30b - Trying to fix the hypo configurations
- 7e332007 - Implement reject empty hypo
- e3e2e161 - Remove printout
- d3db1832 - Updating references
Toggle commit list-
a7064296...4bfb7f00 - 88 commits from branch
-
This merge request affects 4 packages:
- Trigger/TrigHypothesis/TrigTauHypo
- Trigger/TrigValidation/TrigAnalysisTest
- Trigger/TrigValidation/TriggerTest
- Trigger/TriggerCommon/TriggerMenuMT
Affected files list will not be printed in this case
Adding @sutt ,@martindl ,@ademaria ,@vmartin ,@okumura ,@carquin ,@dzanzi ,@bernius ,@hrussell ,@malconad as watchers
added review-pending-level-1 label
-
Athena AthSimulation AthGeneration AnalysisBase AthAnalysis DetCommon externals cmake make required tests optional tests Full details available on this CI monitor view
added urgent label
added review-approved label and removed review-pending-level-1 label
mentioned in commit a36c0287
added sweep:ignore label
