Replace SEP by XX, only capital letters in scenario/prefilter stubs, add jvt prefilter option to HT scenario

As described in the title, the following things were added/changed:

• All letters on an scenario/prefilter stub are now capital letters: dijet -> DIJET, mask -> MASK, etc
• A SEP is no longer required/expected after an scenario/prefilter stub
• Change on our separator (SEP -> XX)
• Optional jvt-based prefilter is now possible in the HT scenario. Example chain added: HLT_j0_pf_ftf_HT50XX010jvt_L1J20
• It is now also possible to use a pt prefilter condition in the HT scenario (extending work done in !42938 (merged))
• Dijet and HT chains using pt or et conditions are added for testing purposes

Regarding the reference files, only changes on chain names are expected (except for a few chains that were added)

ATR-23324

@valentem @peter

changed the description

@jbossios

Hi Jonas,

A number of points about the code in hypoConfigBuilder.py

1. the lines

key = [x for x in prefilter_router if x in pf_string] filters.append(prefilter_routerkey[0])

will create an obscure bug the day the are two keys, one of which contains the other. for example, pf_string = aAAb, with AA and aA in prefilter_router. The routing will then give a wrong result. Which wrong result (prefilter_router['AA'] or prefilter_router['aA']?

If aAAb is also in the router, you might be lucky... until another key is added... hard to tell what the code will do.

2. the lines

• optional

• if groupdict['jvtlo'] is not None:

•    if groupdict['jvthi'] is None:

•        groupdict['jvthi'] = 'inf'

•    vals = defaults('jvt', groupdict['jvtlo'], groupdict['jvthi'])

•    condargs.append(('jvt', deepcopy(vals)))

2a) will not do what I think ypu want to do. I think ypu are trying to set groupdict['jvthi'] = 'inf' is it is missing in the scenario string. Example:

import re

pattern = r'^ht(?P<htlo>\d+)'\
     r'(SEP(?P<etlo>\d*)et(?P<ethi>\d*))?'\
     r'(SEP(?P<etalo>\d*)eta(?P<etahi>\d*))?'\
     r'(SEP(?P<jvtlo>\d*)jvt(?P<jvthi>\d*))?$'

rgx = re.compile(pattern)

ts = ('ht1000', 'ht1000SEP9jvt')
for t in ts:
    m = rgx.match(t)
    print(m)
    gd = m.groupdict()
    print (gd)
    if gd['jvtlo'] is not None:
        if gd['jvthi'] is :
            gd['jvthi'] = 'inf'
    print (gd)
    print()

gives result

<re.Match object; span=(0, 6), match='ht1000'>
{'htlo': '1000', 'etlo': None, 'ethi': None, 'etalo': None, 'etahi': None, 'jvtlo': None, 'jvthi': None}
{'htlo': '1000', 'etlo': None, 'ethi': None, 'etalo': None, 'etahi': None, 'jvtlo': None, 'jvthi': None}

<re.Match object; span=(0, 13), match='ht1000SEP9jvt'>
{'htlo': '1000', 'etlo': None, 'ethi': None, 'etalo': None, 'etahi': None, 'jvtlo': '9', 'jvthi': ''}
{'htlo': '1000', 'etlo': None, 'ethi': None, 'etalo': None, 'etahi': None, 'jvtlo': '9', 'jvthi': ''}

2b the case where only the upper jet level is given should be correctly handled.

Peter

Hi @peter

Thanks for your comments. Please find below some follow-ups:

1. We shouldn't be creating stubs that are that similar so I don't think that ever should/will happen. We made already a similar assumption for the simpler scenario but here and there we could add a check to make sure the length is always 1 to ensure we at least identify such a case if it ever happens (which I will do).

2. I will modify this and also the simpler scenario since the JVT condition actually only needs a single value which is the working point, which we get from jvtlo. So, I will remove jvthi everywhere since it is confusing and not needed at all.

Best, Jona

added 188 commits

• 632d263b...115a7f6e - 186 commits from branch atlas:master
• b8a26b9c - Merge branch 'master' into SupportJVTprefilteringInHTchains
• 9fb6d256 - Remove support to jvthi (HT,simple)

Compare with previous version

changed the description

All of what I mentioned above is now implemented.

added 281 commits

• 9fb6d256...2e44a455 - 277 commits from branch atlas:master
• 129a1896 - None as max default value for jvt (harmonization)
• 0e4ff403 - Merge branch 'master' into SupportJVTprefilteringInHTchains
• 3d482a79 - Revert max jvt in ConditionDefaults.py
• 0264e9da - Scenario stubs all caps, SEP->XX and et->pt on HT

Compare with previous version

changed title from WIP: Remove SEP from mask/ptrange, add jvt prefilter option to HT scenario to WIP: Replace SEP by XX, only capital letters in scenario/prefilter stubs, add jvt prefilter option to HT scenario

changed the description

added 1 commit

• 20e7d10f - Minor change on comment

Compare with previous version

changed the description

added 1 commit

• 0751fc3b - Improve regex for scenario stub identification

Compare with previous version

added 252 commits

• 0751fc3b...74418d1e - 248 commits from branch atlas:master
• b7d6331f - Update offline monitoring config
• e50b0271 - Merge branch 'master' into SupportJVTprefilteringInHTchains
• 5248cd76 - Remove duplicated chains
• c2d56003 - Update reference files

Compare with previous version

marked this merge request as ready

This merge request affects 5 packages:

• Trigger/TrigHypothesis/TrigHLTJetHypo
• Trigger/TrigMonitoring/TrigJetMonitoring
• Trigger/TrigValidation/TrigAnalysisTest
• Trigger/TrigValidation/TriggerTest
• Trigger/TriggerCommon/TriggerMenuMT

Affected files list will not be printed in this case

Adding @sutt ,@dzanzi ,@iriu ,@vmartin ,@okumura ,@carquin ,@ademaria ,@bernius ,@jgeisen ,@hrussell ,@malconad ,@peter ,@jbossios ,@valentem as watchers

added DQ JetEtmiss Trigger TriggerJet TriggerMenu changes-trigger-counts master review-pending-level-1 labels

marked this merge request as draft

CI Result SUCCESS (hash c2d56003)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis	DetCommon
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
DetCommon: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 33092]

Removing the review label as this is marked as draft.

Cheers, Volker (L1)

removed review-pending-level-1 label

added 276 commits

• c2d56003...36d1b384 - 272 commits from branch atlas:master
• 539ebf4f - Merge branch 'master' into SupportJVTprefilteringInHTchains
• cd73e941 - Fix HT using et, place et/pt HT/DIJET examples
• 37a282ab - Merge branch 'master' into SupportJVTprefilteringInHTchains
• 0a19dfa0 - Update reference files

Compare with previous version

changed the description

marked this merge request as ready

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

100K in file Trigger/TrigValidation/TrigAnalysisTest/share/ref_RDOtoRDOTrig_v1Dev_build.ref

This merge request affects 5 packages:

• Trigger/TrigHypothesis/TrigHLTJetHypo
• Trigger/TrigMonitoring/TrigJetMonitoring
• Trigger/TrigValidation/TrigAnalysisTest
• Trigger/TrigValidation/TriggerTest
• Trigger/TriggerCommon/TriggerMenuMT

Affected files list will not be printed in this case

Adding @sutt ,@dzanzi ,@iriu ,@vmartin ,@okumura ,@carquin ,@ademaria ,@bernius ,@jgeisen ,@hrussell ,@malconad ,@peter ,@jbossios ,@valentem as watchers

added review-pending-level-1 label

CI Result SUCCESS (hash 0a19dfa0)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis	DetCommon
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
DetCommon: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 33328]

Changes look like text replacements, regex updating (didn't realize how much our trigger relied on regex!). A part of me can't help but wonder how much downstream analysis code will have to change because of this.

• @peter reviewed the initial version, but a bunch more was added afterward.
• 75% of this is changing tests, and the trigger names in the count files
• 25% is detailed changes to the regex
• So would be more comfortable with another trigger expert check in looking at the final state of the MR.

Marking both for L2 and an expert. L2 feel free to remove expert tag if you are familiar with this already!

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

changed the description

changed the description

Hi Jona,

This looks ok to me - so I agree to the MR.

However, for the record, I think there is a logical problem with the chain names, but I currently think that this is due to the lack if structure in the chain name itself.

So we have the possibility of SCENARIOvariablename (ok) but SCENARIOVariable name (ambiguous). WE have rule that all variable names are lower case, which would resolve the ambiguity: scenario + SCENARIOV, variable = ariabename. But I do not see how we can enforce the rule without an "official" separator. Even using a separator like "SEP" does not fix the problem: how to interpret SCENARIOSEPvarablename? A human would know, but how to check that someone has not comeuppance with a scenario name SCEANARIOSEP? The only way I can see right now is to have an official keyword that acts as a separator. In another context, computing languages have the notion of keywords that cannot be used as say, variable names. There is an official separator for chain names, namely '_', but we are not permitted to use it.

So I think the problem is that there is insufficient structure in the chain name for us to do a clean job. It is doubtful that this will be a problem in practice.

Peter

added 105 commits

• 0a19dfa0...d2989851 - 104 commits from branch atlas:master
• 4191e84c - Merge branch 'master' into SupportJVTprefilteringInHTchains

Compare with previous version

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

100K in file Trigger/TrigValidation/TrigAnalysisTest/share/ref_RDOtoRDOTrig_v1Dev_build.ref

This merge request affects 5 packages:

• Trigger/TrigHypothesis/TrigHLTJetHypo
• Trigger/TrigMonitoring/TrigJetMonitoring
• Trigger/TrigValidation/TrigAnalysisTest
• Trigger/TrigValidation/TriggerTest
• Trigger/TriggerCommon/TriggerMenuMT

Affected files list will not be printed in this case

Adding @sutt ,@dzanzi ,@iriu ,@vmartin ,@okumura ,@carquin ,@ademaria ,@bernius ,@jgeisen ,@hrussell ,@malconad ,@peter ,@jbossios ,@valentem as watchers

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

CI Result FAILURE (hash 4191e84c)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis	DetCommon
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
DetCommon: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 33455]

Jenkins please retry a build

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

100K in file Trigger/TrigValidation/TrigAnalysisTest/share/ref_RDOtoRDOTrig_v1Dev_build.ref

This merge request affects 5 packages:

• Trigger/TrigHypothesis/TrigHLTJetHypo
• Trigger/TrigMonitoring/TrigJetMonitoring
• Trigger/TrigValidation/TrigAnalysisTest
• Trigger/TrigValidation/TriggerTest
• Trigger/TriggerCommon/TriggerMenuMT

Affected files list will not be printed in this case

Adding @sutt ,@dzanzi ,@iriu ,@vmartin ,@okumura ,@carquin ,@ademaria ,@bernius ,@jgeisen ,@hrussell ,@malconad ,@peter ,@jbossios ,@valentem as watchers

CI Result FAILURE (hash 4191e84c)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis	DetCommon
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 1, warnings 22
AthAnalysis: number of compilation errors 0, warnings 0
DetCommon: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 33468]

Jenkins please retry a build

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

100K in file Trigger/TrigValidation/TrigAnalysisTest/share/ref_RDOtoRDOTrig_v1Dev_build.ref

This merge request affects 5 packages:

• Trigger/TrigHypothesis/TrigHLTJetHypo
• Trigger/TrigMonitoring/TrigJetMonitoring
• Trigger/TrigValidation/TrigAnalysisTest
• Trigger/TrigValidation/TriggerTest
• Trigger/TriggerCommon/TriggerMenuMT

Affected files list will not be printed in this case

Adding @sutt ,@dzanzi ,@iriu ,@vmartin ,@okumura ,@carquin ,@ademaria ,@bernius ,@jgeisen ,@hrussell ,@malconad ,@peter ,@jbossios ,@valentem as watchers

CI Result FAILURE (hash 4191e84c)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis	DetCommon
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 1, warnings 11
AthAnalysis: number of compilation errors 0, warnings 0
DetCommon: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 33479]

Jenkins please retry a build

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

100K in file Trigger/TrigValidation/TrigAnalysisTest/share/ref_RDOtoRDOTrig_v1Dev_build.ref

This merge request affects 5 packages:

• Trigger/TrigHypothesis/TrigHLTJetHypo
• Trigger/TrigMonitoring/TrigJetMonitoring
• Trigger/TrigValidation/TrigAnalysisTest
• Trigger/TrigValidation/TriggerTest
• Trigger/TriggerCommon/TriggerMenuMT

Affected files list will not be printed in this case

Adding @sutt ,@dzanzi ,@iriu ,@vmartin ,@okumura ,@carquin ,@ademaria ,@bernius ,@jgeisen ,@hrussell ,@malconad ,@peter ,@jbossios ,@valentem as watchers

CI Result SUCCESS (hash 4191e84c)

	Athena	AthSimulation	AthGeneration	AnalysisBase	AthAnalysis	DetCommon
externals
cmake
make
required tests
optional tests

Full details available on this CI monitor view
Athena: number of compilation errors 0, warnings 0
AthSimulation: number of compilation errors 0, warnings 0
AthGeneration: number of compilation errors 0, warnings 0
AnalysisBase: number of compilation errors 0, warnings 0
AthAnalysis: number of compilation errors 0, warnings 0
DetCommon: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 33518]