Remove old code for checking for truncated HLT result from the IHLTMonTool

The old way of checking for a truncated HLT result no longer works in MT, so the IHLTMonTool code needs to have this code fragment disabled

A comment has been left in since this would need to be fixed properly at some point

Closes #22

This merge request affects 1 package:

• Trigger/TrigMonitoring/TrigHLTMonitoring

Adding @ebergeas as watcher

added Trigger master review-pending-level-1 labels

CI Result SUCCESS (hash daede164)

	Athena	AthSimulation
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
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 9007]

Hi @sutt,
this is discussed in ATR-20835. It's definitely right to say this doesn't work if you run the legacy monitoring over Run-3 trigger output and it tries to check for truncation assuming Run-2 data format. However, this would still work if you run this over legacy trigger output. Although I don't know why the check was there in the first place, I believe we all agreed to leave the legacy monitoring working as it used to on legacy trigger output, and adapt it to in addition work also on Run-3 output, like:

if (run2) {
  // old code
} else {
  // new code
}

Why don't you just set m_ignoreTruncationCheck to true in what you're testing? This would just skip that block of code.

cc @tamartin

Personally, I don't think that code which doesn't even work should be protected from execution by flags to be set by the user in this way - it should be prevented from being run at all until it is fixed.

At the very least, the default value should be set to "true" to skip this check, and the dangerous case (including this check) treated as the special case

Then if the user wants to enable the flag, they can do so in their special case when they know what they are doing, and they know it will not break any code. The default position should always be that it runs and not having a default setting which throws an exception.

Mark

Following TrigDecisionToolMTFeatureAccess I think the way to implement this would be:

if (getTDT()->getNavigationFormat() == "TriggerElement") { // legacy trigger output
  if(getTDT()->ExperimentalAndExpertMethods()->isHLTTruncated()) 
    ATH_MSG_WARNING("HLTResult truncated, skip HLT T0 monitoring for this event");
  else 
    sc = fill();
} else { // Run-3 trigger output
  // no truncation check implemented yet for Run-3 format
  sc = fill();
}

added 1 commit

• 476cd39e - Disable the check on HLT truncation if this is not the legacy monitoring

Compare with previous version

This merge request affects 1 package:

• Trigger/TrigMonitoring/TrigHLTMonitoring

Adding @ebergeas as watcher

added 1 commit

• 7aa5a4b7 - Fix the test for the warning printout

Compare with previous version

This merge request affects 1 package:

• Trigger/TrigMonitoring/TrigHLTMonitoring

Adding @ebergeas as watcher

CI Result SUCCESS (hash 476cd39e)

	Athena	AthSimulation
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
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 9025]

CI Result SUCCESS (hash 7aa5a4b7)

	Athena	AthSimulation
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
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 9026]

resolved all threads

Looks fine for today.

I will aim to address ATR-20835 this-or-next week. And as part of that, I will move this logic out of IHLTMonTool and back into the TDT.

Thanks Tim.

Cheers Mark

the code looks fine from L1 perspective, CI tests passed. Since the discussions with experts have been resolved, I'm approving.

Roman (L1)

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

Thanks Roman

Cheers Mark

merged

mentioned in commit c9bb39cd

added sweep:ignore label

mentioned in merge request !30261 (merged)