Draft: Cleaner way to configure MuonClusterSegmentFinderTool for MuonSegmentFinderAlg

See ATLASRECTS-7325 for details, but I think the current configuration of MuonSegmentFinderAlg is wrong.

This is a proposed fix, but it changes production output and so would need to be discussed in a reco meeting.

cc @mhodgkin @christos

added frozen-tier0-violating label

@emoyse does this remove differences between CA and RecExCommon?

Cheers,

Mark

In principle it shouldn't, since I already tweaked CA to match this when I saw differences. It was only when I dug a bit deeper that I realised there was probably a mistake with old-style. However, having said that the behaviour of old-style is a bit undefined right now, so I will quickly check.

Jenkins please retry a build

This merge request affects 1 package:

• MuonSpectrometer/MuonReconstruction/MuonRecExample

This merge request affects 1 file:

• MuonSpectrometer/MuonReconstruction/MuonRecExample/python/MuonStandalone.py

Adding @apsallid ,@goetz ,@rosati ,@wleight ,@pscholer ,@stavrop ,@jojungge as watchers

added 22.0 MuonSpectrometer labels

added Run3-DataReco-output-changed Run3-MCReco-output-changed labels

CI Result FAILURE (hash d19fd4be)

	Athena	DetCommon
externals
cmake
make
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
DetCommon: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 60297]

Hi

indeed something like this

    Cleaner = getPublicToolClone("MuonTrackCleaner_seg","MuonTrackCleaner")
    Cleaner.Extrapolator = getPublicTool("MuonStraightLineExtrapolator")
    Cleaner.Fitter = getPublicTool("MCTBSLFitterMaterialFromTrack")
    Cleaner.PullCut = 3
    Cleaner.PullCutPhi = 3
    Cleaner.UseSLFit = True

    SegmentFinder = getPublicTool("MuonClusterSegmentFinderTool")

    SegmentFinder.TrackCleaner = Cleaner

re-ordered a bit, will modify the existing instances of the SegmentFinder

If you say that this is not what we need, but we need a fresh copy , I am inclined to trust you. So in everything that is below I assume that from a muon expert prospective we look at a bug, correct?

If the answer is yes. This leaves 2 questions

• The 1st and perhaps which is the important one (@jmaurer , @nstyles ) , we have a small train of muon MRs. I think we need to know an order you would like them to go in.

• As this is not the only one that potentially changes references, and this takes some time. Also this might mean we will time out before we can discuss them all in meetings, so I do not object on discussion in MRs. We can try to be pragmatic if we are convinced we look at bugs.

• But we will need kind of an order for the ones that are real bugs and have to go in asap. Then we can add the "improvement" ones if we do not time out for fast reprocessing. Otherwise I guess is rel 23 for the improvement ones.

The 2nd less importand but still relevant , if is this aligns better old and new config. The one from @mhodgkin

I think we need @sshaw to let us know what the original intention was in order to know the best fix.

I think what you have now looks good, but I'm not sure I'm the best person to ask about the intentions since I pretty much just took what was already there and moved it into a function so we could use it directly in the trigger without duplicating things. I don't think I changed the logic; at least it wasn't my intention to change the logic. In case it helps, from before I refactored things:

https://gitlab.cern.ch/atlas/athena/-/blob/b9acd9a9ef5d168d6591e7dc9d9ddee6b347aab3/MuonSpectrometer/MuonReconstruction/MuonRecExample/python/MuonStandalone.py#L153

Oh man - I should have dug deeper ... indeed this 'mistake' was in the oldest version of the file we have in git: https://gitlab.cern.ch/atlas/athena/-/blob/2d0dccdb25ad66a2ff24715aa6021aaebfbe1a55/MuonSpectrometer/MuonReconstruction/MuonRecExample/python/MuonStandalone.py

So, probably we forget this for now? I would be tempted to leave the ticket open, so we can try the two cases and see if it makes any significant difference to the performance.

P.S. Sorry for falsely blaming you @sshaw!

Jenkins please retry a build

Hi @jmaurer - the CI failures are expected as this changes output and so I'd need to update reference files. But since other FTO changes might be coming, I didn't want to add these until we decide whether this change is needed.

yes, I know. I restarted it to have updated references at hand (needed because of !57598 (merged)) in case this MR gets ready before others.

This merge request affects 1 package:

• MuonSpectrometer/MuonReconstruction/MuonRecExample

This merge request affects 1 file:

• MuonSpectrometer/MuonReconstruction/MuonRecExample/python/MuonStandalone.py

Adding @apsallid ,@goetz ,@rosati ,@wleight ,@pscholer ,@stavrop ,@jojungge as watchers

CI Result FAILURE (hash d19fd4be)

	Athena	DetCommon
externals
cmake
make
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
DetCommon: number of compilation errors 0, warnings 0
For experts only: Jenkins output [CI-MERGE-REQUEST-CC7 60460]

closed

Closing, as this is an OLD issue with Muon config and apparently we managed up until now (this does not mean it's not still a problem, just that it's probably not urgent ... see discussion above)