Skip to content

Add 'parent' to conf2toConfigurable to distinguiish between identically named components

Fix for the case where a CondAlgs's MuonDetectorTool matched to: GeoModelSvc.MuonDetectorTool

Specifically, I saw:

Traceback (most recent call last):
  File "/cvmfs/sft.cern.ch/lcg/releases/LCG_98python3_ATLAS_6/Python/3.7.6/x86_64-centos7-gcc8-opt/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/cvmfs/sft.cern.ch/lcg/releases/LCG_98python3_ATLAS_6/Python/3.7.6/x86_64-centos7-gcc8-opt/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc8-opt/python/TriggerJobOpts/TriggerConfig.py", line 712, in <module>
    appendCAtoAthena( acc )
  File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc8-opt/python/AthenaConfiguration/ComponentAccumulator.py", line 1193, in appendCAtoAthena
    instance = conf2toConfigurable( comp, indent="  " )
  File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc8-opt/python/AthenaConfiguration/ComponentAccumulator.py", line 1150, in conf2toConfigurable
    __setProperties( instance, comp, __indent( indent ) )
  File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc8-opt/python/AthenaConfiguration/ComponentAccumulator.py", line 920, in __setProperties
    setattr( destConfigurableInstance, pname, conf2toConfigurable( pvalue, indent=__indent( indent ) ) )
  File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc8-opt/python/AthenaConfiguration/ComponentAccumulator.py", line 1143, in conf2toConfigurable
    __areSettingsSame( existingConfigurable, comp )
  File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc8-opt/python/AthenaConfiguration/ComponentAccumulator.py", line 1127, in __areSettingsSame
    raise ConfigurationError(err_message)
AthenaConfiguration.ComponentAccumulator.ConfigurationError: Failed merging new config value (1) and old config value (0) for the (FillCacheInitTime) property of MuonDetectorTool/GeoModelSvc.MuonDetectorTool (MuonDetectorTool/MuonDetectorTool) old (new).

Once !41271 (merged) is enabled.

This was very confusing to track down, but it came from this code:

    def __alreadyConfigured( instanceName ):
        from AthenaCommon.Configurable import Configurable
        if instanceName in Configurable.allConfigurables:
            return  Configurable.allConfigurables[instanceName]
        return None

Basically the MuonDetectorTool from MuonDetectorCondAlg was matching to GeoModelSvc.MuonDetectorTool because the instanceName is the same.

My fix is to try to pass the parent through, where needed, so this ambiguity can be resolved. I'm not sure it's a complete fix, but it seems safer than what is there right now.

Edit: I also had to fix the case where I got these errors:

RAWtoESD 02:44:52   File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc8-opt/python/AthenaConfiguration/ComponentAccumulator.py", line 1145, in conf2toConfigurable
RAWtoESD 02:44:52     __setProperties( instance, comp, __indent( indent ) )
RAWtoESD 02:44:52   File "/build2/ci-builds/master/Athena/build/Athena/x86_64-centos7-gcc8-opt/python/AthenaConfiguration/ComponentAccumulator.py", line 931, in __setProperties
RAWtoESD 02:44:52     setattr( destConfigurableInstance, pname, pvalue )
RAWtoESD 02:44:52 
RAWtoESD 02:44:52 ValueError: received an instance of <class 'float'>, but <class 'int'> expected, context: ToolSvc.PixelLayerBuilder.BarrelAdditionalLayerRadii

This was because the correct value (130.) was trying to be added/merged to a configurable with had 130 (i.e. an int). The fix is to pass the correct type in the first place.

Pinging a few people who might want to review this: @wlampl @tbold @tadej @jchapman @elmsheus

Edited by Edward Moyse

Merge request reports