diff --git a/Control/AthenaConfiguration/python/ComponentAccumulator.py b/Control/AthenaConfiguration/python/ComponentAccumulator.py index d48d9ceac08366b0a81f41f0a84cbdf59d520a65..65124a53af8e4d0ee058dfabcde4d24edad25c57 100644 --- a/Control/AthenaConfiguration/python/ComponentAccumulator.py +++ b/Control/AthenaConfiguration/python/ComponentAccumulator.py @@ -939,6 +939,11 @@ def conf2toConfigurable( comp, indent="", suppressDupes=False ): def __areSettingsSame( existingConfigurableInstance, newConf2Instance, indent="" ): _log.debug( "%sChecking if settings are the same %s %s", indent, existingConfigurableInstance.getFullName(), newConf2Instance.getFullJobOptName() ) + if (existingConfigurableInstance.getType() != newConf2Instance.__cpp_type__): + raise ConfigurationError("Old/new ({} | {}) cpp types are not the same for ({} | {}) !".format( + existingConfigurableInstance.getType(),newConf2Instance.__cpp_type__, + existingConfigurableInstance.getFullName(), newConf2Instance.getFullJobOptName() ) ) + alreadySetProperties = existingConfigurableInstance.getValuedProperties().copy() _log.debug( "Existing properties: %s", alreadySetProperties ) _log.debug( "New properties: %s", newConf2Instance._properties ) @@ -950,18 +955,35 @@ def conf2toConfigurable( comp, indent="", suppressDupes=False ): indent, compName(newConf2Instance), pname, compName(pvalue) ) continue propType = newConf2Instance._descriptors[pname].cpp_type - _log.debug( "%sComparing type: %s for: %s", indent, propType, pname ) + _log.debug( "%sComparing type: %s for: %s in: %s", indent, propType, pname, existingConfigurableInstance.getFullJobOptName() ) if "PrivateToolHandleArray" in propType: toolDict = {_.getName(): _ for _ in alreadySetProperties[pname]} _log.debug('Private tool properties? %s', toolDict) newCdict = {_.getName() : _ for _ in pvalue} oldCset = set(toolDict); newCset = set(newCdict) _log.debug('Private tool property names? %s %s', oldCset, newCset) + if ( not (oldCset == newCset) ): + _log.warning('%s PrivateToolHandleArray %s does not have the same named components',indent, pname ) + _log.warning('%s Old (conf1) %s for %s',indent, oldCset, existingConfigurableInstance.getFullJobOptName()) + _log.warning('%s New (conf2) %s for %s',indent, newCset, newConf2Instance.getFullJobOptName()) + _log.warning('%s Will try to merge them, but this might go wrong!',indent) for oldC in oldCset & newCset: __areSettingsSame( toolDict[oldC], newCdict[oldC], __indent(indent)) + # And now just the new properties in conf2 (the stuff just in conf1 is already in the objec) for newC in newCset-oldCset: - # clone new config to old array - alreadySetProperties[pname].append(conf2toConfigurable(newCdict[newC])) + className = newCdict[newC].getFullJobOptName().split( "/" )[0] + _log.debug('%s %s not in oldconfig. Will try to create conf1 instance using this className: %s, and merge.',indent, newC, className) + configurableClass = __findConfigurableClass( className ) + # Do not create with existing name, or it will try to get an existing public tool, if available + # (and public tools cannot be added to a PrivateToolHandleArray) + instance = configurableClass( newC + className + str(len(indent)) ) + + # Now give it the correct name + instance._name = newCdict[newC].name + + __setProperties( instance, newCdict[newC], __indent( indent ) ) + _log.debug('%s will now add %s to array.',indent, instance) + alreadySetProperties[pname].append(instance) elif "PublicToolHandleArray" in propType: toolSet = {_.getName() for _ in alreadySetProperties[pname]} _log.debug('Public tool handle array properties? %s %s', toolSet, pvalue) @@ -982,6 +1004,9 @@ def conf2toConfigurable( comp, indent="", suppressDupes=False ): "skipping deeper checks, configuration may be incorrect", indent, propType, newConf2Instance.name, pname, pvalue) else: + if pvalue is None: + _log.debug("%sThe property value for %s of %s is None. Skipping.", indent, pname, newConf2Instance.name ) + continue _log.debug( "%sSome kind of handle and, object type %s existing %s", indent, type(pvalue), type(existingVal) ) __areSettingsSame( existingVal, pvalue, indent) @@ -991,22 +1016,36 @@ def conf2toConfigurable( comp, indent="", suppressDupes=False ): if pname not in alreadySetProperties: _log.info( "%sAdding property: %s for %s", indent, pname, newConf2Instance.getName() ) - setattr(existingConfigurableInstance, pname, pvalue) + try: + setattr(existingConfigurableInstance, pname, pvalue) + except AttributeError: + _log.info("%s Could not set attribute. Type of existingConfigurableInstance %s.",indent, type(existingConfigurableInstance) ) + raise elif alreadySetProperties[pname] != pvalue: _log.info( "%sMerging property: %s for %s", indent, pname, newConf2Instance.getName() ) # create surrogate clone = newConf2Instance.getInstance("Clone") setattr(clone, pname, alreadySetProperties[pname]) - updatedPropValue = __listHelperToList(newConf2Instance._descriptors[pname].semantics.merge( getattr(newConf2Instance, pname), getattr(clone, pname))) - - setattr(existingConfigurable, pname, updatedPropValue) + try: + updatedPropValue = __listHelperToList(newConf2Instance._descriptors[pname].semantics.merge( getattr(newConf2Instance, pname), getattr(clone, pname))) + except ValueError: + _log.warning( "Failed merging new config value (%s) and old config value (%s) for (%s) property of %s (%s) old (new). Will take value from NEW configuration, but this should be checked!", + getattr(newConf2Instance, pname),getattr(clone, pname),pname,existingConfigurableInstance.getFullJobOptName() ,newConf2Instance.getFullJobOptName() ) + updatedPropValue=getattr(newConf2Instance, pname) + # Necessary because default value is returned for lists... see e.g.: + # https://gitlab.cern.ch/atlas/athena/-/blob/630b82d0540fff24c2a1da6716efc0adb53727c9/Control/AthenaCommon/python/PropertyProxy.py#L109 + _log.debug("existingConfigurable.name: %s, pname: %s, updatedPropValue: %s", existingConfigurableInstance.name(), pname, updatedPropValue ) + + setattr(existingConfigurableInstance, pname, updatedPropValue) del clone _log.info("%s invoked GaudiConf2 semantics to merge the %s and the %s to %s " "for property %s of %s", indent, alreadySetProperties[pname], pvalue, pname, updatedPropValue, existingConfigurable.getFullName()) - + + _log.debug( "%s Conf2 Full name: %s ", indent, comp.getFullJobOptName() ) existingConfigurable = __alreadyConfigured( comp.name ) + if existingConfigurable: # if configurable exists we try to merge with it _log.debug( "%sPre-existing configurable %s was found, checking if has the same properties", indent, comp.getName() ) __areSettingsSame( existingConfigurable, comp ) diff --git a/MuonSpectrometer/MuonConfig/python/MuonRecToolsConfig.py b/MuonSpectrometer/MuonConfig/python/MuonRecToolsConfig.py index 85f2eab75c34b304496d698e3a5eaa96ca15a4f3..3a10fa615b0aa0b7249a180ca18173ad42a38045 100644 --- a/MuonSpectrometer/MuonConfig/python/MuonRecToolsConfig.py +++ b/MuonSpectrometer/MuonConfig/python/MuonRecToolsConfig.py @@ -153,7 +153,7 @@ def MuonAmbiProcessorCfg(flags, name="MuonAmbiProcessor", **kwargs): acc.addPublicTool(scoring_tool) result.merge(acc) kwargs.setdefault('ScoringTool', scoring_tool ) - muon_ami_selection_tool = Muon__MuonAmbiTrackSelectionTool() + muon_ami_selection_tool = Muon__MuonAmbiTrackSelectionTool(name="MuonAmbiSelectionTool") result.addPublicTool(muon_ami_selection_tool) kwargs.setdefault('SelectionTool', muon_ami_selection_tool) result.setPrivateTools(Trk__TrackSelectionProcessorTool(name=name,**kwargs)) @@ -190,7 +190,7 @@ def MuonTrackCleanerCfg(flags, name="MuonTrackCleaner", **kwargs): result.merge( extrapolator_CA ) kwargs.setdefault("Extrapolator", extrapolator) - acc = MCTBFitterCfg(flags, name = "MCTBSLFitterMaterialFromTrack", StraightLine=True, GetMaterialFromTrack=True) + acc = MCTBSLFitterMaterialFromTrackCfg(flags) slfitter = acc.getPrimary() result.merge(acc) kwargs.setdefault("SLFitter", slfitter) @@ -232,17 +232,27 @@ def MuonNavigatorCfg(flags, name="MuonNavigator", **kwargs): result.setPrivateTools(navigator) return result +def MuonStraightLineExtrapolatorCfg(flags, name="MuonStraightLineExtrapolator",**kwargs): + # This is a bit odd , but this is exactly what was in the old configuration + acc = MuonSTEP_PropagatorCfg(flags, name = "MuonStraightLinePropagator") + muon_prop = acc.getPrimary() + kwargs.setdefault("Propagators",[ muon_prop]) + kwargs.setdefault("STEP_Propagator",muon_prop) + result = MuonExtrapolatorCfg(flags, name ,**kwargs) + result.merge (acc) + return result + def MuonExtrapolatorCfg(flags,name = "MuonExtrapolator", **kwargs): Trk__MaterialEffectsUpdator, Trk__EnergyLossUpdator, Trk__MultipleScatteringUpdator=CompFactory.getComps("Trk::MaterialEffectsUpdator","Trk::EnergyLossUpdator","Trk::MultipleScatteringUpdator",) Trk__Extrapolator=CompFactory.Trk.Extrapolator result = ComponentAccumulator() - energy_loss_updator = Trk__EnergyLossUpdator() # Not really sure these should be tools... + energy_loss_updator = Trk__EnergyLossUpdator(name="AtlasEnergyLossUpdator") # Not really sure these should be tools... result.addPublicTool(energy_loss_updator) # TODO remove # This one has a dependency on RndNumberService - mult_scat_updator = Trk__MultipleScatteringUpdator() + mult_scat_updator = Trk__MultipleScatteringUpdator(name="AtlasMultipleScatteringUpdator") result.addPublicTool(mult_scat_updator) # TODO remove material_effects_updator = Trk__MaterialEffectsUpdator( EnergyLossUpdator=energy_loss_updator, MultipleScatteringUpdator=mult_scat_updator) @@ -260,8 +270,8 @@ def MuonExtrapolatorCfg(flags,name = "MuonExtrapolator", **kwargs): muon_prop = acc.getPrimary() result.merge(acc) result.addPublicTool(muon_prop) + kwargs.setdefault("Propagators", [muon_prop]) - kwargs.setdefault("Propagators", [muon_prop]) kwargs.setdefault("ResolveMuonStation", True) kwargs.setdefault("Tolerance", 0.0011) # must be > 1um to avoid missing MTG intersections extrap = Trk__Extrapolator(name=name, **kwargs) @@ -325,7 +335,7 @@ def MuonSTEP_PropagatorCfg(flags, name='MuonSTEP_Propagator', **kwargs): def MCTBExtrapolatorCfg(flags, name='MCTBExtrapolator',**kwargs): result = ComponentAccumulator() - acc = MuonSTEP_PropagatorCfg(flags) + acc = MuonSTEP_PropagatorCfg(flags, name = "MCTBPropagator") prop = acc.getPrimary() result.addPublicTool(prop) result.merge(acc) @@ -337,28 +347,36 @@ def MCTBExtrapolatorCfg(flags, name='MCTBExtrapolator',**kwargs): return result +def MCTBSLFitterMaterialFromTrackCfg(flags, name='MCTBSLFitterMaterialFromTrack', **kwargs): + kwargs["StraightLine"] = True # always set + kwargs["GetMaterialFromTrack"]=True # always set + acc = MuonStraightLineExtrapolatorCfg(flags) + kwargs.setdefault("ExtrapolationTool", acc.getPrimary()) + + propagator = CompFactory.Trk.RungeKuttaPropagator(name="MuonRK_Propagator",AccuracyParameter=0.0002) + kwargs["PropagatorTool"]=propagator + + result = MCTBFitterCfg(flags, name, **kwargs) + result.merge(acc) + return result + def MCTBFitterCfg(flags, name='MCTBFitter', **kwargs): # didn't bother with MCTBSLFitter, since this seems redundant. Just set "StraightLine" = True since the kwargs are passed on to MuonChi2TrackFitterCfg + # Ditto with MCTBFitterMaterialFromTrack. Just set "GetMaterialFromTrack" = True result = ComponentAccumulator() acc = MCTBExtrapolatorCfg(flags) mctbExtrapolator = acc.getPrimary() - result.addPublicTool(mctbExtrapolator) result.merge(acc) kwargs.setdefault("ExtrapolationTool", mctbExtrapolator) kwargs.setdefault("GetMaterialFromTrack", True) kwargs.setdefault("Momentum", flags.Muon.straightLineFitMomentum) - # extra_kwargs = {} - # if 'StraightLine' in kwargs: - # # Pass this on! Can't safely just pass on kwargs, because MuonChi2TrackFitterCfg also has a property ExtrapolationTool - # extra_kwargs.setdefault('StraightLine', kwargs['StraightLine']) - # extra_kwargs.setdefault('GetMaterialFromTrack', kwargs['GetMaterialFromTrack']) acc = MuonChi2TrackFitterCfg(flags, name=name, **kwargs) mctbfitter = acc.getPrimary() result.merge(acc) - # print mctbfitter + result.setPrivateTools(mctbfitter) return result diff --git a/MuonSpectrometer/MuonConfig/python/MuonSegmentFindingConfig.py b/MuonSpectrometer/MuonConfig/python/MuonSegmentFindingConfig.py index f2f2c4ba924e41bd86c300bcbbdf1bbe4fcc0a0d..ec455944705b5b92cac3f57571036f795402b326 100644 --- a/MuonSpectrometer/MuonConfig/python/MuonSegmentFindingConfig.py +++ b/MuonSpectrometer/MuonConfig/python/MuonSegmentFindingConfig.py @@ -24,7 +24,7 @@ Muon__MuonClusterSegmentFinder=CompFactory.getComp("Muon::MuonClusterSegmentFind #Local from MuonConfig.MuonCalibrationConfig import MdtCalibrationDbToolCfg -from MuonConfig.MuonRecToolsConfig import MCTBFitterCfg, MuonAmbiProcessorCfg, MuonStationIntersectSvcCfg, MuonTrackCleanerCfg, MuonTrackSummaryToolCfg, MuonEDMPrinterTool +from MuonConfig.MuonRecToolsConfig import MCTBFitterCfg, MCTBSLFitterMaterialFromTrackCfg, MuonAmbiProcessorCfg, MuonStationIntersectSvcCfg, MuonTrackCleanerCfg, MuonTrackSummaryToolCfg, MuonEDMPrinterTool from MuonConfig.MuonRIO_OnTrackCreatorConfig import MdtCalibWindowNumber def MuonHoughPatternFinderTool(flags, **kwargs): @@ -529,7 +529,7 @@ def MuonClusterSegmentFinderToolCfg(flags, **kwargs): # Won't explicitly configure MuonIdHelperSvc # Won't explicitly configure MuonEDMHelperSvc kwargs.setdefault('SegmentAmbiguityTool', result.popToolsAndMerge( MuonAmbiProcessorCfg(flags) ) ) - kwargs.setdefault('SLFitter', result.popToolsAndMerge( MCTBFitterCfg(flags, name = "SLFitter", StraightLine=True) ) ) + kwargs.setdefault('SLFitter', result.popToolsAndMerge( MCTBSLFitterMaterialFromTrackCfg(flags) ) ) kwargs.setdefault("TrackToSegmentTool", result.popToolsAndMerge( MuonTrackToSegmentToolCfg(flags) ) ) kwargs.setdefault("Printer", MuonEDMPrinterTool(flags) ) kwargs.setdefault('TrackCleaner', result.popToolsAndMerge( MuonTrackCleanerCfg(flags) ) ) diff --git a/Reconstruction/MuonIdentification/MuonCombinedRecExample/python/MuonCombinedRecFlags.py b/Reconstruction/MuonIdentification/MuonCombinedRecExample/python/MuonCombinedRecFlags.py index a86481d3335fceba6555e2e95d41eeb0da0c17db..e7c94f4e2666b5ceff7db58b8afd22c773482406 100644 --- a/Reconstruction/MuonIdentification/MuonCombinedRecExample/python/MuonCombinedRecFlags.py +++ b/Reconstruction/MuonIdentification/MuonCombinedRecExample/python/MuonCombinedRecFlags.py @@ -164,7 +164,11 @@ class createScaleCalibrationInput(JobProperty): allowedTypes=['bool'] StoredValue=False - +## Decide whether to wrap the new configuration in the old. +class useNewConfig(JobProperty): + statusOn=True + allowedTypes=['bool'] + StoredValue=False ## The container with all the flags to steer MuonCombined reconstruction class MuonCombinedRec(JobPropertyContainer): diff --git a/Reconstruction/MuonIdentification/MuonCombinedRecExample/share/MuonCombinedRec_config.py b/Reconstruction/MuonIdentification/MuonCombinedRecExample/share/MuonCombinedRec_config.py index b64199d8fd827cb2cfa0f18df62db176cba8b4c2..8acb7d758d51d52fe268319f15cd210033a0c3a4 100755 --- a/Reconstruction/MuonIdentification/MuonCombinedRecExample/share/MuonCombinedRec_config.py +++ b/Reconstruction/MuonIdentification/MuonCombinedRecExample/share/MuonCombinedRec_config.py @@ -16,9 +16,27 @@ beamFlags = jobproperties.Beam muonCombinedRecFlags.setDefaults() -include ("MuonCombinedRecExample/MuonCombinedRec_preprocessing.py") +if muonCombinedRecFlags.useNewConfig(): + from AthenaCommon.AthenaCommonFlags import athenaCommonFlags + from AthenaConfiguration.AllConfigFlags import ConfigFlags + from AthenaConfiguration.ComponentAccumulator import CAtoGlobalWrapper + from MuonCombinedConfig.MuonCombinedReconstructionConfig import MuonCombinedReconstructionCfg -include ("MuonCombinedRecExample/MuonCombinedRec_identification.py") + ConfigFlags.Input.Files = athenaCommonFlags.FilesInput() + ConfigFlags.Detector.GeometryMDT = True + ConfigFlags.Detector.GeometryTGC = True + ConfigFlags.Detector.GeometryCSC = True + ConfigFlags.Detector.GeometryRPC = True + # TODO Keep here for the moment, since we still have debugging to do. + from AthenaCommon.Logging import logging + log = logging.getLogger( "conf2toConfigurable".ljust(30) ) + log.setLevel(DEBUG) + CAtoGlobalWrapper(MuonCombinedReconstructionCfg,ConfigFlags) -if not rec.doAODMerging(): - include ("MuonCombinedRecExample/MuonCombinedRec_postprocessing.py") +else: + include ("MuonCombinedRecExample/MuonCombinedRec_preprocessing.py") + + include ("MuonCombinedRecExample/MuonCombinedRec_identification.py") + + if not rec.doAODMerging(): + include ("MuonCombinedRecExample/MuonCombinedRec_postprocessing.py")