RNN track classification for HLT taus

Hello,

This MR implements the RNN track classification in the HLT tau reconstruction. For now, we use the json file of the offline RNN training. A quick rates estimate has been done (https://indico.cern.ch/event/862749/), and seems to give good performance. A new configuration called 'tracktwoRNN' schedules the RNN track classification, and also uses the BDT track classifier for FTF core tracks, recently added as 'tracktwoMVABDT'. A new Aux variable has been added in the tau EDM (new entry in an enum), because we need to store the number of charged tracks before RNN track classification, as explained in the talk linked above. In the hypo, we reject candidates that have tracks associated before RNN classification, and which end up as 0p after RNN classification. That way, we keep the possibility to use "real 0p" (candidates for which we loose the track) to recover some efficiency for true taus at low pt and high mu. Some tracktwoRNN chains have been added to the MC_pp_v8 menu for performance studies.

Cheers, Bertrand

added 21.3 Tau Trigger TriggerMenu labels

Tagging @guindon , @malconad , @mmaerker for information.

I am adding the 'urgent' flag, because we hope this MR can make it in time for an upcoming MC performance request (ATR-20469). Thanks a lot.

added urgent label

WARNING: big files (>100K) are found in the changeset

104K in file Trigger/TriggerCommon/TriggerMenu/python/menu/MC_pp_v8.py

This merge request affects 5 packages:

• Event/xAOD/xAODTau
• Reconstruction/tauRecTools
• Trigger/TrigAlgorithms/TrigTauRec
• Trigger/TrigHypothesis/TrigTauHypo
• Trigger/TriggerCommon/TriggerMenu

Adding @goetz ,@martindl ,@rbianchi as watchers

added EDM Reconstruction review-pending-level-1 labels

CI Result FAILURE (hash e3af3062)

	Athena
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 28
For experts only: Jenkins output [CI-MERGE-REQUEST 41400]

The failed test seems to be about the new EDM variable, the T0 test complains about it: http://atlas-computing.web.cern.ch/atlas-computing/links/distDirectory/ci/CIWebArea/nicos_web_areaMRCIbuilds64BS6G62AthenaOpt/NICOS_TestLog_MR-27885-2019-11-11-23-32/ReleaseTests___qTestsTier0_required-test__qTestsTier0_required-test__m.html

nChargedTracksNoMVA is the variable that I added, so it's not surprising to find it there.

Can we add a new variable to 21.3? Escalating to L2. --L1

ERROR    Py:diff-root         INFO  [xAOD::TauJetAuxContainer_v3_HLT_xAOD__TauJetContainer_TrigTauRecMergedAux]: 10 leaves differ

ERROR    Py:diff-root         INFO  [xAOD::TauJetAuxContainer_v3_HLT_xAOD__TauJetContainer_TrigTauRecPreselectionAux]: 5 leaves differ

ERROR    Py:diff-root         INFO  [xAOD::TauJetAuxContainer_v3_TauJetsAux]: 52 leaves differ

ERROR    Your tag breaks the frozen tier0 policy in test q431. 

added review-pending-level-2 label and removed review-pending-level-1 label

Hi @etolley , that's a good question, and I really hope the answer is 'yes' :-) Otherwise it will be impossible to validate the trigger performance correctly.

This is beyond my knowledge... Tagging @rbianchi (?) and escalating to expert review. ~Joe (L2)

added review-pending-expert label and removed review-pending-level-2 label

Hi @jhaley, this is beyond my knowledge as well, sorry. I tag @stelzer and @fwinkl, they will be able to help you or to tag the right people who are able to do so.

Regards, -- Ric.

@okuprash is our trigger EDM coordinator.

Hi @fwinkl, @rbianchi, @jhaley, @etolley,
It is fine to add the variable nChargedTracksNoMVA to the trigger EDM - it is needed to improve performance of the tau trigger, as motivated in the slides at the meeting linked in the description. The reference files for Tier-0 tests would need to be updated.
@martindl, just a reminder, when/if you introduce this also in master, you'll need to list the variable explicitly in TriggerEDMRun3.py.
Cheers,
Oleg

Thanks Oleg! We'll have to validate the performance in 21.3 before porting this new feature to master. Would it be possible to proceed with the MR review, so we can request our MC sample for trigger validation? Thanks a lot.

added review-pending-level-1 label and removed review-pending-expert label

Code changes are fine, adding a new variable to 21.3 was approved by @okuprash, thus approving. Magdalena (L1)

added review-approved label and removed review-pending-level-1 label

This can go in once the the CI reference tests are updated. @martindl, can you run the q-tests locally and put the outputs somewhere public:

RunTier0Tests.py -n -e " --maxEvents=20 "

Then I can copy them to cvmfs.

added review-user-action-required label and removed review-approved label

Hi @jchapman ,

Here is the output: /afs/cern.ch/work/m/martindl/public/RunTier0Tests_forJohn

I copied everything, as I'm not sure what's ultimately needed.

Thanks! Bertrand

Hi @martindl,

Thanks for this. I was just looking at the logs prior to updating the references, when I noticed that the RDOtoRDOTrigger step fails for q221 with the following error:

18:31:11 TrigTauMVA_noMVATES_noTrackBDT_RNN                   INFO TrigTauRecMerged::initialize()
18:31:11 TrigTauMVA_noMVATES_noTrackBDT_RNN                  ERROR  no tools given for this algorithm.
18:31:11 TrigTauMVA_noMVATES_noTrackBDT_RNN                  ERROR Failed to initialize: TrigTauMVA_noMVATES_noTrackBDT_RNN

Full log here:
/afs/cern.ch/work/m/martindl/public/RunTier0Tests_forJohn/run_q221/log.RDOtoRDOTrigger
Sorry that this was missed previously, could you investigate please?
Cheers,
John

Hi @jchapman ,

Hm, I don't quite understand why this happens. I have renamed a trigger sequence from {0}MVATES_{1}TrackBDT_{2}RNN to {0}MVATES_{1}TrackRNN_{2}RNN, because we've never used BDT track classification, while we now use or don't use RNN track classification: !27885 (diffs)

So I am not expecting any {0}MVATES_{1}TrackBDT_{2}RNN to show up with the modified HLT code. It ran successfully before (I was not running this command you pasted above, though).

Maybe I'm not allowed to rename a trigger sequence because it would break some backward compatibility then? It's really a name change, the trigger behaviour is unchanged. I will check if the test is successful when using the previous sequence name.

Trigger experts should correct me, but my naive understanding was that we would keep things unchanged in the v7 list but update the v8 list.

added 1 commit

• 284d50c0 - Use Run2 trigger sequence name for Run2 triggers

Compare with previous version

WARNING: big files (>100K) are found in the changeset

104K in file Trigger/TriggerCommon/TriggerMenu/python/menu/MC_pp_v8.py

This merge request affects 5 packages:

• Event/xAOD/xAODTau
• Reconstruction/tauRecTools
• Trigger/TrigAlgorithms/TrigTauRec
• Trigger/TrigHypothesis/TrigTauHypo
• Trigger/TriggerCommon/TriggerMenu

Adding @goetz ,@martindl ,@rbianchi as watchers

added review-pending-level-1 label and removed review-user-action-required label

I've put back the Run2 trigger sequence name for Run2 triggers. The T0 tests look better. However, there is a large number of diff-root errors that are reported, for almost all objects, not sure what is happening. The updated output can be found here: /afs/cern.ch/work/m/martindl/public/RunTier0Tests_Run2SeqName

Out of curiosity, I ran the test from a fresh area without any code change, I see the same long list of diff-root errors.

Hi @martindl,
Are you running on an SLC6 or a Centos7 machine? This can certainly make a difference to the output. Let's see what the CI results are now.
Cheers,
John

I'm running on a Centos7 machine.

CI Result FAILURE (hash 284d50c0)

	Athena
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 28
For experts only: Jenkins output [CI-MERGE-REQUEST 41621]

Dear all,

That would be really great if we could wrap up the review today, so that we can proceed with the MC performance sample request. Thanks!

Looks good to me, although we would need some expert to comment here, changing labels accordingly...

added review-pending-expert label

removed review-pending-level-1 label

We still need to correctly update the reference files. @martindl can you try to produce the reference files on an SLC6 node - I think that's what the CI runs on for 21.3.

Cheers,

John

Hi John,

Here it is: /afs/cern.ch/work/m/martindl/public/RunTier0Tests_Run2SeqName_slc6. Obtained with 'setupATLAS -c slc6' on Centos7, but I also ran on lxplus6 (much slower), RunTier0Tests.log seems identical.

Cheers, Bertrand

Hi @martindl,

Thanks - I've updated the references on eos. Can you add a commit with the following changes to this MR please?
Change 'v11' to 'v12' here: https://gitlab.cern.ch/atlas/athena/blob/21.3/Tools/PROCTools/python/RunTier0TestsTools.py#L18
Change 'v9' to 'v10' here: https://gitlab.cern.ch/atlas/athena/blob/21.3/Tools/PROCTools/python/RunTier0TestsTools.py#L19

Then the next time that the CI tests run they should hopefully succeed.

Cheers,

John

added 1 commit

• 4d125fe0 - update reference

Compare with previous version

Thanks! Done.

WARNING: big files (>100K) are found in the changeset

104K in file Trigger/TriggerCommon/TriggerMenu/python/menu/MC_pp_v8.py

This merge request affects 6 packages:

• Event/xAOD/xAODTau
• Reconstruction/tauRecTools
• Tools/PROCTools
• Trigger/TrigAlgorithms/TrigTauRec
• Trigger/TrigHypothesis/TrigTauHypo
• Trigger/TriggerCommon/TriggerMenu

Adding @goetz ,@martindl ,@adbailey ,@amete ,@rbianchi as watchers

added Tools review-pending-level-1 labels and removed review-pending-expert label

CI Result FAILURE (hash 4d125fe0)

	Athena
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 31
For experts only: Jenkins output [CI-MERGE-REQUEST 41648]

Good, a new error :-) It seems we're hit by !28108 (merged).