Enabling use of cached Views in precision Calo step

This MR enables use of Cached Views in precisionCalo step on top of changes applied to precisionTrackingHypo. The step counts for probe legs for two cases

chainComp INFO       HLT_e26_lhtight_ivarloose_e5_lhtight_probe_L1EM22VHI:
chainComp INFO         stepCounts:
chainComp INFO           7: 4 -> 3
chainComp INFO           8: 4 -> 3
chainComp INFO         stepFeatures:
chainComp INFO           8: 11 -> 10
chainComp INFO           9: 5 -> 4
chainComp INFO       HLT_e26_lhtight_ivarloose_e5_lhvloose_idperf_probe_L1EM22VHI:
chainComp INFO         eventCount: 5 -> 4
chainComp INFO         stepCounts:
chainComp INFO           7: 5 -> 4
chainComp INFO           8: 5 -> 4
chainComp INFO         stepFeatures:
chainComp INFO           8: 14 -> 13

but as discussed with @tamartin this may be that we are passing some events in error while this flag (precisionCaloViewsMaker.CacheDisabled) is true i.e. we have one candidate, and we reconstruct it twice (because we are not re-using the cached result), and then we go and pass a two-electron chain with just one electron (for example). As I locally checked with a signal sample to see how much reduction we get in probe leg by removing the flag

athena.py --imf --perfmon --threads 1 --concurrent-events 1 --filesInput '/eos/home-d/dbakshig/data17_13TeV.00339070.physics_Main/data17_13TeV.00339070.physics_Main.merge.DRAW_EGZ.f887_m1831._0152.1' --evtMax 50 --skipEvents 0 -c 'from AthenaConfiguration.AllConfigFlags import ConfigFlags;ConfigFlags.Trigger.disableCPS=True;doEmptyMenu=True;doEgammaSlice=True;isOnline=True;doWriteBS=False;doWriteRDOTrigger=True;forceEnableAllChains=True;selectChains=["HLT_e26_lhtight_ivarloose_e5_lhtight_probe_L1EM22VHI"];setGlobalTag="CONDBR2-HLTP-2018-01"' TriggerJobOpts/runHLT_standalone.py 2>&1 |tee electron_ManyEvents.log

I don't see any reduction. So propose its best to move this change for upcoming 19th round of validation and we will get to see how much its affecting in large no. of sample (adding @cjmeyer , @jodafons )

added urgent label

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

152K in file Trigger/TrigValidation/TrigP1Test/share/ref_v1Dev_decodeBS_build.ref

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

This merge request affects 5 packages:

• Trigger/TrigHypothesis/TrigEgammaHypo
• Trigger/TrigHypothesis/TrigStreamerHypo
• Trigger/TrigValidation/TrigAnalysisTest
• Trigger/TrigValidation/TrigP1Test
• Trigger/TriggerCommon/TriggerMenuMT

Affected files list will not be printed in this case

Adding @sutt ,@amorley ,@dbakshig ,@salderwe ,@vmartin ,@okumura ,@jodafons ,@fernando ,@bernius ,@hrussell ,@cjmeyer ,@dzanzi as watchers

added Egamma Trigger TriggerMenu changes-trigger-counts master review-pending-level-1 labels

CI Result SUCCESS (hash 4ff50348)

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

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
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 46226]

Thanks @dbakshig,

The number of accepted events does not change, only the step in which we apply the rejection.

This is because CacheDisabled = True means that we re-reconstruct instead of re-using. Hence the ComboHypo isn't able to tell that a given candidate was reconstructed twice on this Step and erroneously lets one candidate pass a two-electron chain down at this probe step.

The ComboHypo can tell that they're identical on the next step (where we use caching again), so the overall physics result is unaffected.

Overall - good to remove this CacheDisabled = True flag given it's no longer needed to solve another issue.

FYI, this has caught merge conflicts, which need to be resolved. -L1

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

:( this is not proving an easy change to get in...

added 120 commits

• 4ff50348...4532bc27 - 117 commits from branch atlas:master
• 2e26a408 - removing commented line
• 7d2bacbe - Merge tag 'nightly/master/2022-02-02T2101' into enablingCachedView_02_02_22
• 60328a49 - Updating references

Compare with previous version

resolved all threads

changed the description

added 66 commits

• 60328a49...ba687ee6 - 64 commits from branch atlas:master
• 02c7c5e6 - Merge tag 'nightly/master/2022-02-03T2101' into enablingCachedView_02_02_22
• c8aaa429 - Updating references

Compare with previous version

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

156K in file Trigger/TrigValidation/TrigP1Test/share/ref_v1Dev_decodeBS_build.ref

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

This merge request affects 5 packages:

• Trigger/TrigHypothesis/TrigEgammaHypo
• Trigger/TrigHypothesis/TrigStreamerHypo
• Trigger/TrigValidation/TrigAnalysisTest
• Trigger/TrigValidation/TrigP1Test
• Trigger/TriggerCommon/TriggerMenuMT

Affected files list will not be printed in this case

Adding @sutt ,@amorley ,@dbakshig ,@salderwe ,@vmartin ,@okumura ,@jodafons ,@fernando ,@bernius ,@hrussell ,@cjmeyer ,@dzanzi as watchers

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

CI Result FAILURE (hash c8aaa429)

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

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
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 46316]

We only have some partial printout, but the issue is the number of messages

We see things such as

From A:HLTNav_IMprecisionTracking:18 Dump:
TrigComposite_v1 name:'IM'
N Links:4, isRemapped:NO
Link Name:seed__COLL, Key:1014972503, Index:18, CLID:1333228823
Link Name:seed__COLL, Key:96321906, Index:2, CLID:1333228823
Link Name:roi, Key:1065038136, Index:14, CLID:1097199488
Link Name:view, Key:104798376, Index:12, CLID:1160627009
N Decisions:2
324908483, 3015571664,
From B:HLTNav_IMprecisionTracking:16 Dump:
TrigComposite_v1 name:'IM'
N Links:4, isRemapped:NO
Link Name:seed__COLL, Key:1014972503, Index:16, CLID:1333228823
Link Name:seed__COLL, Key:96321906, Index:0, CLID:1333228823
Link Name:roi, Key:1065038136, Index:14, CLID:1097199488
Link Name:view, Key:104798376, Index:12, CLID:1160627009
N Decisions:2

which comes from here https://acode-browser1.usatlas.bnl.gov/lxr/source/athena/Trigger/TrigSteer/TrigCompositeUtils/TrigCompositeUtils/TrigCompositeUtils.icc#0073

It will be the "view" link which is getting flagged here.

And it's totally expected to re-use views here.

@dbakshig

I think it will be overall quicker to apply this change here vs another MR and then re-starting the CI.

Could you update this one line here https://acode-browser1.usatlas.bnl.gov/lxr/source/athena/Trigger/TrigSteer/TrigCompositeUtils/TrigCompositeUtils/TrigCompositeUtils.icc#0063 to be

if (li.source != start && linkName != initialRoIString() && linkName != roiString() && linkName != "l2cbroi" && linkName != viewString()) {

I.e. we add "view" to the exclusion list and don't print this WARNING here.

added 1 commit

• 54d1ecdd - Removing view related warnings

Compare with previous version

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

156K in file Trigger/TrigValidation/TrigP1Test/share/ref_v1Dev_decodeBS_build.ref

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

This merge request affects 6 packages:

• Trigger/TrigHypothesis/TrigEgammaHypo
• Trigger/TrigHypothesis/TrigStreamerHypo
• Trigger/TrigSteer/TrigCompositeUtils
• Trigger/TrigValidation/TrigAnalysisTest
• Trigger/TrigValidation/TrigP1Test
• Trigger/TriggerCommon/TriggerMenuMT

Affected files list will not be printed in this case

Adding @sutt ,@amorley ,@dbakshig ,@salderwe ,@vmartin ,@okumura ,@jodafons ,@fernando ,@bernius ,@tamartin ,@hrussell ,@cjmeyer ,@dzanzi as watchers

added analysis-review-required label

CI Result SUCCESS (hash 54d1ecdd)

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

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
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 46331]

Merge conflict due to reference updates over the weekend: !50263 (merged)

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

This MR is proving to be very conflict-prone, @khoo & @dzanzi are we OK to prioritise this one among menu MRs? (Noting that the one on the weekend which conflicted wasn't from the trigger)

Yes, let's do that.

added RC Attention Required label

mentioned in merge request !50235 (merged)

As r06 is broken, we will need r07 before we can update the refs here (following from the RNG update)

mentioned in merge request !50172 (merged)

mentioned in merge request !49843 (merged)

added analysis-review-approved label and removed analysis-review-required label

added 169 commits

• 54d1ecdd...b531b586 - 166 commits from branch atlas:master
• 78891274 - Merge tag 'nightly/master/2021-02-05T2101' into enablingCachedView_02_02_22
• 72a0af33 - Merge tag 'nightly/master/2022-02-08T0246' into enablingCachedView_02_02_22
• feb6ec1e - Updating references

Compare with previous version

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

156K in file Trigger/TrigValidation/TrigP1Test/share/ref_v1Dev_decodeBS_build.ref

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

This merge request affects 6 packages:

• Trigger/TrigHypothesis/TrigEgammaHypo
• Trigger/TrigHypothesis/TrigStreamerHypo
• Trigger/TrigSteer/TrigCompositeUtils
• Trigger/TrigValidation/TrigAnalysisTest
• Trigger/TrigValidation/TrigP1Test
• Trigger/TriggerCommon/TriggerMenuMT

Affected files list will not be printed in this case

Adding @sutt ,@amorley ,@dbakshig ,@salderwe ,@vmartin ,@okumura ,@jodafons ,@fernando ,@bernius ,@tamartin ,@hrussell ,@cjmeyer ,@dzanzi as watchers

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

CI Result SUCCESS (hash feb6ec1e)

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

Full details available on this CI monitor view. Check the JIRA CI status board for known problems
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 46553]

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

resolved all threads

GitLab fails to squash this. @dbakshig, for the future please make sure to use the rebase (instead of merge) workflow: https://atlassoftwaredocs.web.cern.ch/gittutorial/branch-and-change/#keeping-your-branch-up-to-date

Exceptionally, I will merge this without squashing to avoid further delays.

mentioned in commit 27285703

merged

Many thanks, @fwinkl!

mentioned in merge request !50284 (merged)

added sweep:ignore label

mentioned in merge request !50325 (merged)