Add an option to use the angular distance instead of cartesian for IsolatedHitMergingAlgorithm

[SanghyunKo]

Add an option to use the angular distance instead of cartesian for IsolatedHitMergingAlgorithm. Stripped from #33 as IsolatedHitMergingAlgorithm is being used in CLD's PandoraSettingsDefault.xml.

Validated that the behavior is bit-by-bit identical when using the default values for the new options, using k4GaudiPandora's run_Pandora_ttbar CI workflow and the following comparison script (similar to the existing compare-pfos.py script, but Gaudi vs Gaudi instead marlin vs Gaudi).

compare-gaudi-identical.py (click to expand)

#!/usr/bin/env python
#
# Copyright (c) 2020-2024 Key4hep-Project.
#
# This file is part of Key4hep.
# See https://key4hep.github.io/key4hep-doc/ for further info.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

# A simple script to check that the Pandora output of two Gaudi files is
# identical, value-by-value. Intended to verify that a change (e.g. an
# LCContent commit) does not alter the reconstruction output at all.
#
# Note: two ROOT files are never byte-identical even for identical data
# (they embed per-write UUIDs/timestamps and compression can differ), so a
# raw `cmp` is meaningless. Instead we decode every collection and compare
# every data member with exact `==` (bit-exact for floats) and every
# relation by its ObjectID index, following the structure of compare-pfos.py.
import argparse
import sys
from podio.reading import get_reader

parser = argparse.ArgumentParser(
    description="Check that the Pandora output of two Gaudi files is identical"
)
parser.add_argument(
    "--file-before",
    default="output_pandora_ttbar_before.root",
    help="Gaudi output file produced before the change",
)
parser.add_argument(
    "--file-after",
    default="output_pandora_ttbar.root",
    help="Gaudi output file produced after the change",
)
parser.add_argument(
    "--vertices-collections",
    default=["GaudiPandoraStartVertices"],
    nargs="+",
    help="Vertices collections to compare",
)
parser.add_argument(
    "--cluster-collections",
    default=["GaudiPandoraClusters"],
    nargs="+",
    help="Cluster collections to compare",
)
parser.add_argument(
    "--recoparticle-collections",
    default=["GaudiPandoraPFOs"],
    nargs="+",
    help="Reconstructed particle collections to compare",
)

args = parser.parse_args()

reader_before = get_reader(args.file_before)
reader_after = get_reader(args.file_after)

events_before = reader_before.get("events")
events_after = reader_after.get("events")

assert len(events_before) == len(events_after), (
    f"Number of events differ: {len(events_before)} (before) "
    f"vs {len(events_after)} (after)"
)

for i, frame_before in enumerate(events_before):
    frame_after = events_after[i]

    for collection in args.vertices_collections:
        print(f'Checking vertices collection "{collection}" in event {i}')
        vertices_before = frame_before.get(collection)
        vertices_after = frame_after.get(collection)
        assert len(vertices_before) == len(vertices_after), (
            f"Number of vertices differ for {collection}: "
            f"{len(vertices_before)} vs {len(vertices_after)}"
        )
        for j, (vertex_before, vertex_after) in enumerate(
            zip(vertices_before, vertices_after)
        ):
            for attr in [
                "Type",
                "Chi2",
                "Ndf",
                "Position",
                "CovMatrix",
                "AlgorithmType",
            ]:
                assert (
                    getattr(vertex_before, f"get{attr}")()
                    == getattr(vertex_after, f"get{attr}")()
                ), f"{attr} differ for vertex {j}: {getattr(vertex_before, f'get{attr}')()} vs {getattr(vertex_after, f'get{attr}')()}"

            for vmember in [
                "Parameters",
            ]:
                assert list(getattr(vertex_before, f"get{vmember}")()) == list(
                    getattr(vertex_after, f"get{vmember}")()
                ), f"{vmember} differ for vertex {j}: {list(getattr(vertex_before, f'get{vmember}')())} vs {list(getattr(vertex_after, f'get{vmember}')())}"

            for relation in [
                "Particles",
            ]:
                assert [
                    elem.id().index
                    for elem in getattr(vertex_before, f"get{relation}")()
                ] == [
                    elem.id().index
                    for elem in getattr(vertex_after, f"get{relation}")()
                ], f"{relation} differ for vertex {j}"

    for collection in args.cluster_collections:
        print(f'Checking cluster collection "{collection}" in event {i}')
        clusters_before = frame_before.get(collection)
        clusters_after = frame_after.get(collection)
        assert len(clusters_before) == len(clusters_after), (
            f"Number of clusters differ for {collection}: "
            f"{len(clusters_before)} vs {len(clusters_after)}"
        )
        for j, (cluster_before, cluster_after) in enumerate(
            zip(clusters_before, clusters_after)
        ):
            for attr in [
                "Type",
                "Energy",
                "EnergyError",
                "Position",
                "PositionError",
                "ITheta",
                "Phi",
                "DirectionError",
            ]:
                assert (
                    getattr(cluster_before, f"get{attr}")()
                    == getattr(cluster_after, f"get{attr}")()
                ), f"{attr} differ for cluster {j}: {getattr(cluster_before, f'get{attr}')()} vs {getattr(cluster_after, f'get{attr}')()}"

            for vmember in [
                "ShapeParameters",
                "SubdetectorEnergies",
            ]:
                assert list(getattr(cluster_before, f"get{vmember}")()) == list(
                    getattr(cluster_after, f"get{vmember}")()
                ), f"{vmember} differ for cluster {j}: {list(getattr(cluster_before, f'get{vmember}')())} vs {list(getattr(cluster_after, f'get{vmember}')())}"

            for relation in [
                "Clusters",
                "Hits",
            ]:
                assert [
                    elem.id().index
                    for elem in getattr(cluster_before, f"get{relation}")()
                ] == [
                    elem.id().index
                    for elem in getattr(cluster_after, f"get{relation}")()
                ], f"{relation} differ for cluster {j}"

    for collection in args.recoparticle_collections:
        print(f'Checking reconstructed particle collection "{collection}" in event {i}')
        recos_before = frame_before.get(collection)
        recos_after = frame_after.get(collection)
        assert len(recos_before) == len(recos_after), (
            f"Number of reconstructed particles differ for {collection}: "
            f"{len(recos_before)} vs {len(recos_after)}"
        )
        for j, (reco_before, reco_after) in enumerate(zip(recos_before, recos_after)):
            for attr in [
                "PDG",
                "Energy",
                "Momentum",
                "ReferencePoint",
                "Charge",
                "Mass",
                "GoodnessOfPID",
                "CovMatrix",
            ]:
                assert (
                    getattr(reco_before, f"get{attr}")()
                    == getattr(reco_after, f"get{attr}")()
                ), f"{attr} differ for reco {j}: {getattr(reco_before, f'get{attr}')()} vs {getattr(reco_after, f'get{attr}')()}"

            for relation in [
                "DecayVertex",
            ]:
                assert (
                    getattr(reco_before, f"get{relation}")().id().index
                    == getattr(reco_after, f"get{relation}")().id().index
                ), f"{relation} differ for reco {j}"

            for relation in [
                "Clusters",
                "Tracks",
                "Particles",
            ]:
                assert [
                    elem.id().index for elem in getattr(reco_before, f"get{relation}")()
                ] == [
                    elem.id().index for elem in getattr(reco_after, f"get{relation}")()
                ], f"{relation} differ for reco {j}"

print(
    f"\nOK: the two files are identical over {len(events_before)} event(s) for "
    f"collections {args.vertices_collections + args.cluster_collections + args.recoparticle_collections}"
)
sys.exit(0)

[andresailer]

Looks generally good. Just some minor nitpicking.

[andresailer]

Either align all on the = or don't align any.

[SanghyunKo]

Fixed (now align all =)

[andresailer]

Why not go for a range based loop here, when you change the variable anyway? Although I don't understand why the variable name was changed.

[SanghyunKo]

Now I use the range-based loop here (indeed there was no reason to use iterator here)

[andresailer]

Why change the return type of this function? Am I missing where this does not return STATUS_CODE_SUCCESS?

[SanghyunKo]

Now this function returns float same as before.

[andresailer]

Thanks!