Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Move MLAmbiguitySolver to Core #3272

Open
wants to merge 60 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 53 commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
a5aa62a
fix network size issue
Corentin-Allaire Apr 3, 2024
8e4d55c
spacepoint writter
Corentin-Allaire May 31, 2024
99e1fb5
Move to Core, removed DBScan version
Corentin-Allaire Jun 11, 2024
cdae466
Merge remote-tracking branch 'upstream/main' into ML-to-Core
Corentin-Allaire Jun 11, 2024
de38b17
format
Corentin-Allaire Jun 11, 2024
31e24e1
format
Corentin-Allaire Jun 11, 2024
2f1e3cc
format
Corentin-Allaire Jun 11, 2024
2ca678c
format
Corentin-Allaire Jun 11, 2024
7737d09
updated the doc
Corentin-Allaire Jun 11, 2024
ab07a56
rename
Corentin-Allaire Jun 11, 2024
bb67645
fix P in CsvSpacePointWriter
Corentin-Allaire Jun 11, 2024
4ceb1bd
forgot to update include Onnx.cpp
Corentin-Allaire Jun 11, 2024
da5a458
forgot to update include Onnx.cpp
Corentin-Allaire Jun 11, 2024
130960a
remove cout
Corentin-Allaire Jul 9, 2024
3f1738a
uncommented line
Corentin-Allaire Aug 26, 2024
d7e1157
Merge remote-tracking branch 'upstream/main' into ML-to-Core
Corentin-Allaire Aug 28, 2024
c6e3c62
added ML solver to CI
Corentin-Allaire Aug 28, 2024
eb87b39
lint
Corentin-Allaire Aug 28, 2024
bf61491
lint
Corentin-Allaire Aug 28, 2024
a3b6874
.parent
Corentin-Allaire Aug 28, 2024
28656ae
Merge remote-tracking branch 'upstream/main' into ML-to-Core
Corentin-Allaire Aug 28, 2024
7390e7c
file naming
Corentin-Allaire Aug 28, 2024
d1bd61e
.root
Corentin-Allaire Aug 28, 2024
57f141b
Merge branch 'main' into ML-to-Core
Corentin-Allaire Aug 29, 2024
5b87fe4
reco.py
Corentin-Allaire Sep 25, 2024
555e10c
added a concept for the network
Corentin-Allaire Sep 27, 2024
bfe3b31
Merge remote-tracking branch 'upstream/main' into ML-to-Core
Corentin-Allaire Sep 27, 2024
496b1fc
Merge remote-tracking branch 'origin/ML-to-Core' into ML-to-Core
Corentin-Allaire Sep 27, 2024
68434dd
format
Corentin-Allaire Sep 27, 2024
147cb2c
p
Corentin-Allaire Sep 27, 2024
69651e3
fix CI
Corentin-Allaire Sep 27, 2024
85077d9
update
Corentin-Allaire Nov 19, 2024
410077d
licence
Corentin-Allaire Nov 20, 2024
f45b867
licence
Corentin-Allaire Nov 20, 2024
9b660f7
licence
Corentin-Allaire Nov 20, 2024
1866f64
issue in writter
Corentin-Allaire Nov 20, 2024
95250ed
reference fo ml solver
Corentin-Allaire Nov 20, 2024
1d5feef
Merge remote-tracking branch 'upstream/main' into ML-to-Core
Corentin-Allaire Nov 20, 2024
64f870d
matching for solver writter
Corentin-Allaire Nov 20, 2024
2a085d5
more documentation, move the ML solver at the end of the chain
Corentin-Allaire Nov 20, 2024
2c2bed0
fix CI ?
Corentin-Allaire Nov 20, 2024
a70878e
missing option for ML Solver
Corentin-Allaire Nov 20, 2024
8fae08d
hopefully fixing the CI
Corentin-Allaire Nov 20, 2024
97b1c35
hopefully fixing the CI
Corentin-Allaire Nov 20, 2024
9e79097
Physmon ref
Corentin-Allaire Nov 21, 2024
2f8e11a
Physmon fix
Corentin-Allaire Nov 21, 2024
4135c0e
Merge remote-tracking branch 'upstream/main' into ML-to-Core
Corentin-Allaire Nov 21, 2024
fa8d97b
adressing comments
Corentin-Allaire Nov 21, 2024
dc21c86
Merge remote-tracking branch 'upstream/main' into ML-to-Core
Corentin-Allaire Nov 21, 2024
e949fe8
remove measurement from loop
Corentin-Allaire Nov 23, 2024
21be966
Merge remote-tracking branch 'upstream/main' into ML-to-Core
Corentin-Allaire Nov 23, 2024
e6d93cb
Merge branch 'main' into ML-to-Core
CarloVarni Nov 25, 2024
f311a2b
remove unnecessary part of concept
Corentin-Allaire Nov 26, 2024
ada0ef7
Merge remote-tracking branch 'upstream/main' into ML-to-Core
Corentin-Allaire Nov 26, 2024
4e07f5c
Merge branch 'main' into ML-to-Core
Corentin-Allaire Nov 27, 2024
ea981bb
Update Examples/Io/Csv/src/CsvSpacePointWriter.cpp
Corentin-Allaire Nov 27, 2024
c2f7b12
fix broken loop
Corentin-Allaire Nov 28, 2024
abcaf43
Merge remote-tracking branch 'origin/ML-to-Core' into ML-to-Core
Corentin-Allaire Nov 28, 2024
d3c9e60
Merge remote-tracking branch 'upstream/main' into ML-to-Core
Corentin-Allaire Nov 28, 2024
6b2cc82
update physmon file
Corentin-Allaire Nov 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CI/physmon/phys_perf_mon.sh
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,15 @@ function trackfinding() {
$path/performance_finding_ckf_ambi.html \
$path/performance_finding_ckf_ambi
fi

if [ -f $refdir/$path/performance_finding_ckf_ml_solver.root ]; then
run_histcmp \
$outdir/data/$path/performance_finding_ckf_ml_solver.root \
$refdir/$path/performance_finding_ckf_ml_solver.root \
"ML Ambisolver | ${name}" \
$path/performance_finding_ckf_ml_solver.html \
$path/performance_finding_ckf_ml_solver
fi
}

function vertexing() {
Expand Down
Binary file not shown.
Binary file not shown.
27 changes: 27 additions & 0 deletions CI/physmon/workflows/physmon_trackfinding_ttbar_pu200.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
CkfConfig,
addCKFTracks,
addAmbiguityResolution,
addAmbiguityResolutionML,
AmbiguityResolutionConfig,
AmbiguityResolutionMLConfig,
addVertexFitting,
VertexFinder,
TrackSelectorConfig,
Expand Down Expand Up @@ -134,13 +136,25 @@
outputDirRoot=tp,
)

addAmbiguityResolutionML(
s,
AmbiguityResolutionMLConfig(
maximumSharedHits=3, maximumIterations=1000000, nMeasurementsMin=6
),
tracks="ckf_tracks",
outputDirRoot=tp,
onnxModelFile=Path(__file__).resolve().parent.parent.parent.parent
/ "thirdparty/OpenDataDetector/data/duplicateClassifier.onnx",
)

Corentin-Allaire marked this conversation as resolved.
Show resolved Hide resolved
addAmbiguityResolution(
s,
AmbiguityResolutionConfig(
maximumSharedHits=3,
maximumIterations=100000,
nMeasurementsMin=6,
),
tracks="ckf_tracks",
Corentin-Allaire marked this conversation as resolved.
Show resolved Hide resolved
outputDirRoot=tp,
)

Expand Down Expand Up @@ -187,6 +201,17 @@
tp / "performance_fitting_ambi.root",
tp / "performance_fitting_ckf_ambi.root",
)

shutil.move(
tp / "performance_finding_ambiML.root",
tp / "performance_finding_ckf_ml_solver.root",
)

shutil.move(
tp / "performance_fitting_ambiML.root",
tp / "performance_fitting_ckf_ml_solver.root",
)

for vertexing in ["amvf_gauss_notime", "amvf_grid_time"]:
shutil.move(
tp / f"{vertexing}/performance_vertexing.root",
Expand All @@ -200,6 +225,8 @@
"performance_fitting_ckf.root",
"performance_finding_ckf_ambi.root",
"performance_fitting_ckf_ambi.root",
"performance_finding_ckf_ml_solver.root",
"performance_fitting_ckf_ml_solver.root",
"performance_vertexing_amvf_gauss_notime.root",
"performance_vertexing_amvf_grid_time.root",
]:
Expand Down
58 changes: 58 additions & 0 deletions Core/include/Acts/AmbiguityResolution/AmbiguityNetworkConcept.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// This file is part of the ACTS project.
//
// Copyright (C) 2016 CERN for the benefit of the ACTS project
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

#pragma once

#include "Acts/EventData/TrackContainer.hpp"
#include "Acts/EventData/TrackContainerFrontendConcept.hpp"
#include "Acts/EventData/VectorMultiTrajectory.hpp"
#include "Acts/EventData/VectorTrackContainer.hpp"
#include "Acts/Utilities/Concepts.hpp"

namespace Acts {

using DummyTrackContainer =
TrackContainer<VectorTrackContainer, VectorMultiTrajectory,
detail::ValueHolder>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be in the global namespace at this level with this kind of name.


/// @brief Concept for the ambiguity network used in the ambiguity resolution
///
/// The ambiguity network correspond to the AmbiguityTrackClassifier found in
/// the Onnx plugin. It is used to score the tracks and select the best ones.
///
/// The constructor of the Ambiguity Solver network should take string as input
/// corresponding to the path of the ONNX model.
/// The implementation of the Ambiguity Solver network should have two methods:
/// - inferScores: takes clusters (a list of track ID associated with a cluster
/// ID) and the track container and return an outputTensor (list of scores for
/// each track in the clusters).
/// - trackSelection: Takes clusters and the output tensor from the inferScores
/// method and return the list of track ID to keep.
///
/// @tparam N the type of the network
template <typename network_t>
concept AmbiguityNetworkConcept =
TrackContainerFrontend<DummyTrackContainer> &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems redundant, what is this checking?

requires(
DummyTrackContainer &tracks,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be the only real case where you need the typedef, can you just inline the current typedef here directly?

Suggested change
DummyTrackContainer &tracks,
TrackContainer<VectorTrackContainer, VectorMultiTrajectory,
detail::ValueHolder> &tracks,

std::unordered_map<std::size_t, std::vector<std::size_t>> &clusters,
std::vector<std::vector<float>> &outputTensor, const char *modelPath,
network_t &n) {
requires TrackContainerFrontend<DummyTrackContainer>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also seems redundant, you're checking it above, and I don't think this needs to be verified for this combination of type arguments.


{ network_t(modelPath) } -> std::same_as<network_t>;

{
n.inferScores(clusters, tracks)
} -> std::same_as<std::vector<std::vector<float>>>;
{
n.trackSelection(clusters, outputTensor)
} -> std::same_as<std::vector<std::size_t>>;
};

} // namespace Acts
136 changes: 136 additions & 0 deletions Core/include/Acts/AmbiguityResolution/AmbiguityResolutionML.hpp
Corentin-Allaire marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
// This file is part of the ACTS project.
//
// Copyright (C) 2016 CERN for the benefit of the ACTS project
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

#pragma once

#include "Acts/AmbiguityResolution/AmbiguityNetworkConcept.hpp"
#include "Acts/Definitions/Units.hpp"
#include "Acts/EventData/TrackContainer.hpp"
#include "Acts/Utilities/Delegate.hpp"
#include "Acts/Utilities/Logger.hpp"

#include <cstddef>
#include <map>
#include <memory>
#include <string>
#include <tuple>
#include <vector>

namespace Acts {

/// Generic implementation of the machine learning ambiguity resolution
/// Contains method for data preparations
template <AmbiguityNetworkConcept AmbiguityNetwork>
class AmbiguityResolutionML {
public:
struct Config {
/// Path to the model file for the duplicate neural network
std::string inputDuplicateNN = "";
/// Minimum number of measurement to form a track.
std::size_t nMeasurementsMin = 7;
};
/// Construct the ambiguity resolution algorithm.
///
/// @param cfg is the algorithm configuration
/// @param logger is the logging instance
AmbiguityResolutionML(const Config& cfg,
std::unique_ptr<const Logger> logger = getDefaultLogger(
"AmbiguityResolutionML", Logging::INFO))
: m_cfg{cfg},
m_duplicateClassifier(m_cfg.inputDuplicateNN.c_str()),
Corentin-Allaire marked this conversation as resolved.
Show resolved Hide resolved
m_logger{std::move(logger)} {}

/// Associate the hits to the tracks
///
/// This algorithm performs the mapping of hits ID to track ID. Our final goal
/// is too loop over all the tracks (and their associated hits) by order of
/// decreasing number hits for this we use a multimap where the key is the
/// number of hits as this will automatically perform the sorting.
///
/// @param tracks is the input track container
/// @param sourceLinkHash is the hash function for the source link, will be used to associate to tracks
/// @param sourceLinkEquality is the equality function for the source link used used to associated hits to tracks
/// @return an ordered list containing pairs of track ID and associated measurement ID
template <TrackContainerFrontend track_container_t,
typename source_link_hash_t, typename source_link_equality_t>
std::multimap<int, std::pair<std::size_t, std::vector<std::size_t>>>
mapTrackHits(const track_container_t& tracks,
const source_link_hash_t& sourceLinkHash,
const source_link_equality_t& sourceLinkEquality) const {
// A map to store (and generate) the measurement index for each source link
auto measurementIndexMap =
std::unordered_map<SourceLink, std::size_t, source_link_hash_t,
source_link_equality_t>(0, sourceLinkHash,
sourceLinkEquality);

// A map to store the track Id and their associated measurements ID, a
// multimap is used to automatically sort the tracks by the number of
// measurements
std::multimap<int, std::pair<std::size_t, std::vector<std::size_t>>>
trackMap;
Comment on lines +74 to +75
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure tracks are sorted in decreasing order

By default, in ascending order std::multimap sorts. To sort tracks by decreasing number of measurements, a custom comparator provide, you must.

std::size_t trackIndex = 0;
std::vector<std::size_t> measurements;
// Loop over all the trajectories in the events
for (const auto& track : tracks) {
// Kick out tracks that do not fulfill our initial requirements
if (track.nMeasurements() < m_cfg.nMeasurementsMin) {
continue;
}
measurements.clear();
for (auto ts : track.trackStatesReversed()) {
if (ts.typeFlags().test(Acts::TrackStateFlag::MeasurementFlag)) {
SourceLink sourceLink = ts.getUncalibratedSourceLink();
// assign a new measurement index if the source link was not seen yet
auto emplace = measurementIndexMap.try_emplace(
sourceLink, measurementIndexMap.size());
measurements.push_back(emplace.first->second);
}
}
trackMap.emplace(track.nMeasurements(),
std::make_pair(trackIndex, measurements));
++trackIndex;
}
return trackMap;
}

/// Select the track associated with each cluster
///
/// In this algorithm the call the neural network to score the tracks and then
/// select the track with the highest score in each cluster
///
/// @param clusters is a map of clusters, each cluster correspond to a vector of track ID
/// @param tracks is the input track container
/// @return a vector of trackID corresponding tho the good tracks
template <TrackContainerFrontend track_container_t>
std::vector<std::size_t> solveAmbiguity(
std::unordered_map<std::size_t, std::vector<std::size_t>>& clusters,
const track_container_t& tracks) const {
std::vector<std::vector<float>> outputTensor =
m_duplicateClassifier.inferScores(clusters, tracks);
std::vector<std::size_t> goodTracks =
m_duplicateClassifier.trackSelection(clusters, outputTensor);

return goodTracks;
}

private:
// Configuration
Config m_cfg;

// The neural network for duplicate classification, the network
// implementation is chosen with the AmbiguityNetwork template parameter
AmbiguityNetwork m_duplicateClassifier;

/// Logging instance
std::unique_ptr<const Logger> m_logger = nullptr;

/// Private access to logging instance
const Logger& logger() const { return *m_logger; }
};

} // namespace Acts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,15 @@

namespace Acts::detail {

/// Clusterise tracks based on shared hits
/// Cluster tracks based on shared hits.
///
/// In this algorithm we will loop through all the tracks by decreasing number
/// of measurements. Cluster are created when a new track is encountered that
/// doesn't share hits with the leading track of a previous cluster (with the
/// leading track defined as the track that lead to the cluster creation). If a
/// track shares hits with the leading track of a cluster, it is added to that
/// cluster. If a track shares hits with multiple clusters, it is associated to
/// the cluster with the leading track with the most hits.
///
/// @param trackMap : Multimap storing pair of track ID and vector of measurement ID. The keys are the number of measurement and are just there to facilitate the ordering.
/// @return an unordered map representing the clusters, the keys the ID of the primary track of each cluster and the store a vector of track IDs.
Expand Down
2 changes: 0 additions & 2 deletions Examples/Algorithms/TrackFindingML/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
set(SOURCES
src/AmbiguityResolutionML.cpp
src/AmbiguityResolutionMLAlgorithm.cpp
src/AmbiguityResolutionMLDBScanAlgorithm.cpp
src/SeedFilterMLAlgorithm.cpp
)

Expand Down

This file was deleted.

Loading
Loading