Master add flag convertor and fixes

As discussed in various meetings, it would be useful to have standardised conversion from old-to-new flags, at least for the most basic ones

This is a first attempt, so comments are welcomed. Please note that even with recent MRs improving the defaults for NewConfig, we use RecExCommon in many workflows and so we cannot rely on defaults - newconfig must match oldconfig.

As agreed, I also made mismatches a FATAL error (and throw an exception) to avoid having misconfigured new config overwriting correctly configured oldconfig.

Tagging a few people who might be interested: @ponyisi @goetz @mhodgkin @elmsheus @wlampl @fwinkl @tbold @tadej @nkoehler

added NewConfig full-unit-tests labels

This merge request affects 3 packages:

• Control/AthenaConfiguration
• Reconstruction/MuonIdentification/MuonCombinedRecExample
• Reconstruction/RecExample/RecExCommon

Affected files list will not be printed in this case

Adding @goetz ,@rosati ,@wleight ,@ssnyder ,@jojungge as watchers

added Core Reconstruction master review-pending-level-1 labels

changed the description

Looks good to me, but I'm still afraid that we do not set this in all places that we should.

No I’m sure we don’t. This isn’t supposed to be complete - I just wanted to get what I had in as a first step (or get comments for improvements).

Hi Ed - looks good to me, agree to having a first version in then taking updates. It seems to cover the non-DQ parts of what we use. Do you think we should put system-specific flag config in here? In additional functions that getNewConfigFlags would call?

CI Result FAILURE (hash 3abf8c1a)

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

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

This is related to: ATR-21276 - it seems we can use this in trigger context as well.

added 4 commits

• 1622dcce - More fixes for conf2toConfigurable.
• 8f3b6b03 - getNewConfigFlags: hide false positive flake8 warning
• c320c240 - Set default DetFlags from GeoModel geometry
• 664c764c - Attempt to fix the TGC cabling new configuration to match the old configuration.

Compare with previous version

This merge request affects 5 packages:

• Control/AthenaConfiguration
• MuonSpectrometer/MuonConfig
• MuonSpectrometer/MuonReconstruction/MuonRecExample
• Reconstruction/MuonIdentification/MuonCombinedRecExample
• Reconstruction/RecExample/RecExCommon

Affected files list will not be printed in this case

Adding @goetz ,@rosati ,@wleight ,@jojungge ,@ssnyder as watchers

added MuonSpectrometer label

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

marked this merge request as draft

CI Result FAILURE (hash 664c764c)

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

added 4 commits

• 979acfdc - Add OldFlags2NewFlags
• 7a37b46c - Use OldFlags2NewFlags in CombinedRec, muon combined and AthenaMonitoring.
• 3ff8df41 - More fixes for conf2toConfigurable.
• ca8e7d13 - More fixes for new/old config mismatch for muons.

Compare with previous version

Hi @ponyisi, the test DataQuality_required.citest was failing because of:

RDOtoRDOTrigger 22:28:49 AthenaConfiguration.ComponentAccumulator.ConfigurationError: ('Failed merging new config value (%s) and old config value (%s) for (%s) property of %s (%s) old (new).', True, False, 'forcedUse', 'TGCcablingServerSvc/TGCcablingServerSvc', 'TGCcablingServerSvc/TGCcablingServerSvc')

So I think this is an example of DQ changing reco config (no idea if it had any impact in practice). It should be fixed now.

I did a rebase and force pushed, so I'm afraid I've messed up the comment history of this MR a bit. But I didn't want to restart a new MR as I don't have much time today...

resolved all threads

marked this merge request as ready

removed review-user-action-required label

This merge request affects 6 packages:

• Control/AthenaConfiguration
• Control/AthenaMonitoring
• MuonSpectrometer/MuonConfig
• MuonSpectrometer/MuonReconstruction/MuonRecExample
• Reconstruction/MuonIdentification/MuonCombinedRecExample
• Reconstruction/RecExample/RecExCommon

Affected files list will not be printed in this case

Adding @goetz ,@rosati ,@wleight ,@jojungge ,@ssnyder as watchers

added DQ MuonSpectrometer Trigger review-pending-level-1 labels

CI Result FAILURE (hash ca8e7d13)

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

Unit tests failures must be unrelated:

input_line_71:1:10: fatal error: 'xAODMuon/MuonContainer.h' file not found
#include "xAODMuon/MuonContainer.h"

The SimulationTier0Test_required-test also looks suspicious.

grepping errors in run_s3505/log.EVNTtoHITS
grep: run_s3505/log.EVNTtoHITS: No such file or directory

Jenkins please retry a build

resolved all threads

This merge request affects 6 packages:

• Control/AthenaConfiguration
• Control/AthenaMonitoring
• MuonSpectrometer/MuonConfig
• MuonSpectrometer/MuonReconstruction/MuonRecExample
• Reconstruction/MuonIdentification/MuonCombinedRecExample
• Reconstruction/RecExample/RecExCommon

Affected files list will not be printed in this case

Adding @goetz ,@rosati ,@wleight ,@jojungge ,@ssnyder as watchers

CI Result FAILURE (hash ca8e7d13)

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

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

@jchapman - do you have any bright ideas about this failure:

ESDtoAOD 18:07:42 Py:CaloRingerOutputItemList_jobOptions.py    INFO Entering
ESDtoAOD 18:07:42 Py:Athena            INFO including file "AthenaMonitoring/DataQualitySteering_jobOptions.py"
ESDtoAOD 18:07:42 Py:DataQualitySteering_jobOptions    INFO DQ: setting up ConfigFlags
ESDtoAOD 18:07:42 Shortened traceback (most recent user call last):
ESDtoAOD 18:07:42   File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc8-opt/jobOptions/RecJobTransforms/skeleton.ESDtoAOD_tf.py", line 102, in <module>
ESDtoAOD 18:07:42     else: include( "RecExCommon/RecExCommon_topOptions.py" )
ESDtoAOD 18:07:42   File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc8-opt/jobOptions/RecExCommon/RecExCommon_topOptions.py", line 1627, in <module>
ESDtoAOD 18:07:42     protectedInclude ("AthenaMonitoring/DataQualitySteering_jobOptions.py")
ESDtoAOD 18:07:42   File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc8-opt/jobOptions/AthenaMonitoring/DataQualitySteering_jobOptions.py", line 335, in <module>
ESDtoAOD 18:07:42     ConfigFlags = getNewConfigFlags()
ESDtoAOD 18:07:42   File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc8-opt/python/AthenaConfiguration/OldFlags2NewFlags.py", line 45, in getNewConfigFlags
ESDtoAOD 18:07:42     ConfigFlags._set('Detector.Geometry'+flag, getattr(DetFlags.detdescr,geom_flag_map[flag]+'_on')())
ESDtoAOD 18:07:42   File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc8-opt/python/AthenaConfiguration/AthConfigFlags.py", line 243, in _set
ESDtoAOD 18:07:42     raise KeyError(errString)
ESDtoAOD 18:07:42 
ESDtoAOD 18:07:42 KeyError: "No flag with name 'Detector.GeometryBpipe' found"
ESDtoAOD 18:07:42 

Detector.GeometryBpipe obviously does exist: https://gitlab.cern.ch/atlas/athena/-/blob/master/Control/AthenaConfiguration/python/DetectorConfigFlags.py#L61

I'm trying to reproduce this locally.

Edit: Ah. Maybe it needs to be dynamically loaded first. I bet this is it.

Detector flags are lazy loaded. You might need ConfigFlags._loadDynaFlags('Detector').

Thanks @tadej! Yep. I worked it out (see my edit above) ... (though I don't quite understand what function I should call to make this happen automatically ... but _loadDynaFlags is good enough. Now I've a real problem to fix...

added 1 commit

• 71db7b8a - Force load the Detector flags. Use haveRIO instead of makeRIO, sp ESDtoAOD step works.

Compare with previous version

This merge request affects 6 packages:

• Control/AthenaConfiguration
• Control/AthenaMonitoring
• MuonSpectrometer/MuonConfig
• MuonSpectrometer/MuonReconstruction/MuonRecExample
• Reconstruction/MuonIdentification/MuonCombinedRecExample
• Reconstruction/RecExample/RecExCommon

Affected files list will not be printed in this case

Adding @goetz ,@rosati ,@wleight ,@jojungge ,@ssnyder as watchers

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

changed the description

CI Result FAILURE (hash 71db7b8a)

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

Failure seems unrelated.

resolved all threads

The CI failures indeed seem to be unrelated, but to make sure I would like L2 to take a look as well.

Cheers, Volker (L1)

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

The CI failures are unrelated. Approving.

Cheers, Abhishek [L2]

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

Hi @emoyse , could you please fix merge conflicts for this one? Thanks.

added 307 commits

• 71db7b8a...46f32b2c - 302 commits from branch atlas:master
• 230a151b - Failure to merge a config value is no longer a warning, but now raises a...
• d6bb5a3e - Add OldFlags2NewFlags
• f9a08978 - Use OldFlags2NewFlags in CombinedRec, muon combined and AthenaMonitoring.
• 6f3ddac4 - More fixes for conf2toConfigurable.
• 2937a012 - More fixes for new/old config mismatch for muons.

Compare with previous version

This merge request affects 5 packages:

• Control/AthenaConfiguration
• Control/AthenaMonitoring
• MuonSpectrometer/MuonReconstruction/MuonRecExample
• Reconstruction/MuonIdentification/MuonCombinedRecExample
• Reconstruction/RecExample/RecExCommon

Affected files list will not be printed in this case

Adding @goetz ,@rosati ,@wleight ,@ssnyder ,@jojungge as watchers