Add track merging algorithm based on track parameter compatibility

[Victor-Schwan]

BEGINRELEASENOTES

• Adds a new TrackMerger Gaudi transformer that merges tracks from two input collections based on compatible track-state parameters at their adjoining hit
• While generic and applicable to merging any two track collections, it was designed to combine silicon-tracker tracks with TPC tracks reconstructed by Clupatra for ILD
• The exact matching criterion in track-parameter space is not yet optimized and may need tuning
• Includes self-contained CTest test_TrackMerger* that generates synthetic matching/non-matching track pairs and verifies correct merging behavior

ENDRELEASENOTES

[jmcarcell]

Why not call k4run from CMakeLists.txt like all the tests everywhere else do? Also you can have a steering file and modify parameters.

[Victor-Schwan]

My idea was to have a self-contained script.

1. All data of my test is in a temporary directory and after execution all files are removed and the git repository is in a clean state. The other tests do not remove their files and litter my repository.
2. The script creates its own input and steering files -> no need to download input files or steering files as it is in the other tests
3. The test_trackFitter.sh basically only calls the python script. I am not aware of a significant advantage that the wrapping offers
4. You can modify parameters in my python script as well. Parameters like the tolerances for merging and collection names, are located at the top of the script for convenient adaptation

Of course, one can split the different steps in this python script over several files but why?

[jmcarcell]

You can just add the files to .gitignore and you will never notice the files from git. Indeed there is no reason to have test_trackFitter.sh instead of calling directly the k4run command; I just see it now. Creating the input file is fine and for steering files we already have a mechanism to change parameters instead of having to generate it; this also allows it to be run independently instead of having to find out the python script, then the generation, then which command runs, it's all much more complex than needed for a simple test.

[Victor-Schwan]

Ok. Do you think there are more adaptations necessary than promoting the steering script to a standalone file? This results in outsourcing step 2 of the python script but the rest remains as it is?

[Victor-Schwan]

Compared to the other tests, the track merger test has to check whether one track pair was merged and the other pair not. AFAIK the other tests pass as long as the processors were executed without any problem. This verification step, the input file generation and the execution would remain in the python script.

[jmcarcell]

Ok. Do you think there are more adaptations necessary than promoting the steering script to a standalone file? This results in outsourcing step 2 of the python script but the rest remains as it is?

That should be enough, that almost forces you to have two CMake tests, one for running and another one for checking, you can make the check depend on the running passing with CMake fixtures. This is done in k4FWCore if you want to see some examples.

Compared to the other tests, the track merger test has to check whether one track pair was merged and the other pair not. AFAIK the other tests pass as long as the processors were executed without any problem. This verification step, the input file generation and the execution would remain in the python script.

Ideally they should check their output too, they also pass if they process 0 events

[Victor-Schwan]

Should be solved by the last commit. Based on your suggestion, I've created two tests that depend on each other via fixtures, and a cleanup "test".

[jmcarcell]

But this does not call k4run from CMakeLists which was my original comment, isn't that simpler than calling subprocess? Then the logs are stored in a file and you don't need to do

    print(result.stdout[-3000:])  # tail to keep output readable

What if the failure happens in characters that are removed? Now you have to run it, ah where is the command... inside the script, where I think no one would expect it. Also if you ever want to use this script outside of this repo there are many changes that have to be done because of the complex setup, instead of just taking input and output files.

[Victor-Schwan]

ok, the truncated-stdout issue alone could've been fixed by just dropping capture_output=True (then everything flows through to CTest's log), but I agree the k4run command being hidden inside the script makes reproducing failures annoying. I've now split it into three CTest steps chained via fixtures:

1. python setup (writes input file)
2. k4run called directly from CMakeLists
3. python check

The steering file only takes --input/--output, so it's also reusable outside the repo. If you'd rather keep it consistent with the other tests, I could alternatively wrap setup + k4run in a small bash script but I do not see an advantage of that.

Are you fine with this update?

[jmcarcell]

It seems I forgot to write this: the results of this algorithm depend on the ordering of the input tracks, which is not nice. Also, this is only useful for something downstream that wants the hits but instead if you return a link between each pair of tracks then someone could have available all the information and you don't have to create new Track objects.

[Victor-Schwan]

1. On the dependence of track ordering:
   This is only the case if the greedy option is activated. I suggest that we change the default to deactivate this option. Then, the default behavior does not depend on the ordering. We can provide a warning and comment that this option introduces a dependence on track ordering. I discussed with Thomas that we would like to study this option. Hence, I would like to keep it in the code. Is that fine?
2. On providing the links:
   What about adding an option which writes out the links? If someone is interested in the links, they can activate this option.

[jmcarcell]

Ok for 1. Also greedy typically means taking the best option at each step (https://en.wikipedia.org/wiki/Greedy_algorithm), which would be to find the "best" track for each input track and using that one even if globally it's not the best matching. Slightly different from first that passes but I'm not sure which word reflects that better.
2. Well actualy downstream algorithms would need to work with links instead of Tracks so it's also not the best setup so I guess Tracks is fine.

[Victor-Schwan]

IIUC points 1) and 2) are solved. If not, please let me know

[jmcarcell]

IIUC points 1) and 2) are solved. If not, please let me know

I don't see any new commits

[Victor-Schwan]

I don't see any new commits

That is because there are no new commits. I understood your replies as follows:

1. Ok for 1. [...] Slightly different from first that passes but I'm not sure which word reflects that better.

Since neither of us has a better idea for a more precise wording, I assumed we are fine with leaving it as it is

2. [...] so I guess Tracks is fine

Currently, the code writes out tracks with the merged hit collections. Previously, you had thought about writing out links for a downstream algo but you changed your mind and thought that tracks (the way it is currently implemented) is fine. Therefore I assumed you were fine with leaving it as it is.

---

Hence, I concluded there is no need for any changes/new commits. Please let me know what I have misunderstood :)

[jmcarcell]

By the way, what do we do with #72?

[Victor-Schwan]

By the way, what do we do with #72?

Good question. @tmadlener was pushing for merging this, so I would like to discuss that with him when he is back which will be soon. This will certainly not affect this PR. The question will be whether we add #72 in addition, in a modified version or not at all.

[jmcarcell]

Unrelated CI failures (related to key4hep/key4hep-spack#898)