Skip to content

Commit

Permalink
Merge branch 'acts-project:main' into main
Browse files Browse the repository at this point in the history
  • Loading branch information
zzalscv2 authored Jun 17, 2024
2 parents a4c1d2c + 7e1e819 commit e23732e
Show file tree
Hide file tree
Showing 25 changed files with 1,077 additions and 119 deletions.
20 changes: 10 additions & 10 deletions CONTRIBUTING.rst
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ Bug reports and feature requests
--------------------------------

To report an issue and before starting work, please create an issue in the
`GitHub issue tracker <https://github.com/acts-project/acts-core/issues>`_. A
`GitHub issue tracker <https://github.com/acts-project/acts/issues>`_. A
comprehensive explanation will help the development team to respond in a timely
manner. Verbalising the issue before starting work allows the other contributors
to chime in and avoids disagreements how to progress.
Expand Down Expand Up @@ -76,10 +76,10 @@ be merged in, request a review from the `reviewers team
request is reviewed, it can be merged in.

To get started with git, please refer to the `short introduction
<http://git-scm.com/docs/gittutorial>`_ as well as the `full git documentation
<https://git-scm.com/docs/gittutorial>`_ as well as the `full git documentation
<https://git-scm.com/doc>`_. Tutorials as well as explanations of concepts and
workflows with git can also be found on `Atlassian
<http://www.atlassian.com/git/>`_.
<https://www.atlassian.com/git/>`_.

Checklist for pull requests
~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down Expand Up @@ -138,8 +138,8 @@ familiar with development process in the Acts project.
Well-written commit messages are key
to understand your changes. There are many guidelines available on
how to write proper commit logs (e.g.
`here <http://alistapart.com/article/the-art-of-the-commit>`__,
`here <http://chris.beams.io/posts/git-commit/>`__, or
`here <https://alistapart.com/article/the-art-of-the-commit>`__,
`here <https://cbea.ms/git-commit/>`__, or
`here <https://wiki.openstack.org/wiki/GitCommitMessages#Information_in_commit_messages>`__).
As a short summary:

Expand All @@ -162,9 +162,9 @@ familiar with development process in the Acts project.
remote repository. These merge commits are considered to contribute
little information to the development process of the feature and they
clutter the history (read more e.g.
`here <https://developer.atlassian.com/blog/2016/04/stop-foxtrots-now/>`__
`here <https://www.atlassian.com/blog/it-teams/stop-foxtrots-now>`__
or
`here <http://victorlin.me/posts/2013/09/30/keep-a-readable-git-history>`__).
`here <https://fangpenlin.com/posts/2013/09/30/keep-a-readable-git-history/>`__).
This problem can be avoided by using ``git pull --rebase`` which
replays your local (un-pushed) commits on the tip of the remote
branch. You can make this the default behaviour by running
Expand All @@ -182,7 +182,7 @@ Coding style and guidelines
~~~~~~~~~~~~~~~~~~~~~~~~~~~

The Acts project uses
`clang-format <http://clang.llvm.org/docs/ClangFormat.html>`_ for
`clang-format <https://clang.llvm.org/docs/ClangFormat.html>`_ for
formatting its source code. A ``.clang-format`` configuration file comes
with the project and should be used to automatically format the code.
Developers can use the provided Docker image to format their project or
Expand All @@ -192,7 +192,7 @@ installing `the same clang version as used in the
CI <https://github.com/acts-project/machines/blob/master/format10/Dockerfile>`_
is recommended. There are several instructions available on how to
integrate clang-format with your favourite IDE (e.g. `Xcode <https://github.com/travisjeffery/ClangFormat-Xcode>`_,
`emacs <http://clang.llvm.org/docs/ClangFormat.html#emacs-integration>`_).
`emacs <https://clang.llvm.org/docs/ClangFormat.html#emacs-integration>`_).
The Acts CI system will automatically check code formatting using the
provided clang-format configuration and will notify incompatible formatting.

Expand Down Expand Up @@ -269,7 +269,7 @@ member of the reviewers team before being merged but anyone is welcome
to contribute by commenting on code changes. You can help reviewing
proposed contributions by going to `the "pull requests" section of the
Acts (core) GitHub
repository <https://github.com/acts-project/acts-core/pulls>`_.
repository <https://github.com/acts-project/acts/pulls>`_.

As some of the guidelines recommended here require rights granted to the
reviewers team, this guide specifically addresses the people in this
Expand Down
21 changes: 0 additions & 21 deletions Core/include/Acts/Surfaces/CylinderBounds.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,25 +159,4 @@ inline bool CylinderBounds::coversFullAzimuth() const {
return m_closed;
}

inline void CylinderBounds::checkConsistency() noexcept(false) {
if (get(eR) <= 0.) {
throw std::invalid_argument("CylinderBounds: invalid radial setup.");
}
if (get(eHalfLengthZ) <= 0.) {
throw std::invalid_argument("CylinderBounds: invalid length setup.");
}
if (get(eHalfPhiSector) <= 0. || get(eHalfPhiSector) > M_PI) {
throw std::invalid_argument("CylinderBounds: invalid phi sector setup.");
}
if (get(eAveragePhi) != detail::radian_sym(get(eAveragePhi))) {
throw std::invalid_argument("CylinderBounds: invalid phi positioning.");
}
if (get(eBevelMinZ) != detail::radian_sym(get(eBevelMinZ))) {
throw std::invalid_argument("CylinderBounds: invalid bevel at min Z.");
}
if (get(eBevelMaxZ) != detail::radian_sym(get(eBevelMaxZ))) {
throw std::invalid_argument("CylinderBounds: invalid bevel at max Z.");
}
}

} // namespace Acts
14 changes: 14 additions & 0 deletions Core/include/Acts/Surfaces/CylinderSurface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "Acts/Surfaces/SurfaceConcept.hpp"
#include "Acts/Utilities/BinningType.hpp"
#include "Acts/Utilities/Concepts.hpp"
#include "Acts/Utilities/Logger.hpp"
#include "Acts/Utilities/Result.hpp"
#include "Acts/Utilities/detail/RealQuadraticEquation.hpp"

Expand Down Expand Up @@ -247,6 +248,19 @@ class CylinderSurface : public RegularSurface {
ActsMatrix<2, 3> localCartesianToBoundLocalDerivative(
const GeometryContext& gctx, const Vector3& position) const final;

/// Merge two cylinder surfaces into a single one.
/// @image html Cylinder_Merging.svg
/// @note The surfaces need to be *compatible*, i.e. have cylinder bounds
/// that align, and have the same radius
/// @param gctx The current geometry context object, e.g. alignment
/// @param other The other cylinder surface to merge with
/// @param direction The binning direction: either @c binZ or @c binRPhi
/// @param logger The logger to use
/// @return The merged cylinder surface
std::shared_ptr<CylinderSurface> mergedWith(
const GeometryContext& gctx, const CylinderSurface& other,
BinningValue direction, const Logger& logger = getDummyLogger()) const;

protected:
std::shared_ptr<const CylinderBounds> m_bounds; //!< bounds (shared)

Expand Down
13 changes: 13 additions & 0 deletions Core/include/Acts/Surfaces/DiscSurface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,19 @@ class DiscSurface : public RegularSurface {
ActsMatrix<2, 3> localCartesianToBoundLocalDerivative(
const GeometryContext& gctx, const Vector3& position) const final;

/// Merge two disc surfaces into a single one.
/// @image html Disc_Merging.svg
/// @note The surfaces need to be *compatible*, i.e. have disc bounds
/// that align
/// @param gctx The current geometry context object, e.g. alignment
/// @param other The other disc surface to merge with
/// @param direction The binning direction: either @c binR or @c binPhi
/// @param logger The logger to use
/// @return The merged disc surface
std::shared_ptr<DiscSurface> mergedWith(
const GeometryContext& gctx, const DiscSurface& other,
BinningValue direction, const Logger& logger = getDummyLogger()) const;

protected:
std::shared_ptr<const DiscBounds> m_bounds; ///< bounds (shared)
};
Expand Down
31 changes: 31 additions & 0 deletions Core/include/Acts/Surfaces/detail/MergeHelper.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// This file is part of the Acts project.
//
// Copyright (C) 2024 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 http://mozilla.org/MPL/2.0/.

#pragma once

#include "Acts/Definitions/Algebra.hpp"
#include "Acts/Definitions/Tolerance.hpp"
#include "Acts/Definitions/Units.hpp"
#include "Acts/Utilities/Logger.hpp"
#include "Acts/Utilities/detail/periodic.hpp"

#include <utility>

namespace Acts::detail {

/// This helper function calculates the combination
/// of two phi sectors, defined by a phi half-length +
/// a half phi sector in the range [0,pi). The two
/// ranges need to line up, i.e. that one of the sector
/// ends exactly where the other one starts.
std::pair<ActsScalar, ActsScalar> mergedPhiSector(
ActsScalar hlPhi1, ActsScalar avgPhi1, ActsScalar hlPhi2,
ActsScalar avgPhi2, const Logger& logger = getDummyLogger(),
ActsScalar tolerance = s_onSurfaceTolerance);

} // namespace Acts::detail
113 changes: 46 additions & 67 deletions Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// This file is part of the Acts project.
//
// Copyright (C) 2023 CERN for the benefit of the Acts project
// Copyright (C) 2023-2024 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
Expand Down Expand Up @@ -49,9 +49,14 @@ namespace Acts::Experimental {

namespace Gx2fConstants {
constexpr std::string_view gx2fnUpdateColumn = "Gx2fnUpdateColumn";

// Mask for the track states. We don't need Smoothed and Filtered
constexpr TrackStatePropMask trackStateMask = TrackStatePropMask::Predicted |
TrackStatePropMask::Jacobian |
TrackStatePropMask::Calibrated;
} // namespace Gx2fConstants

/// Extension struct which holds delegates to customize the KF behavior
/// Extension struct which holds delegates to customise the GX2F behaviour
template <typename traj_t>
struct Gx2FitterExtensions {
using TrackStateProxy = typename MultiTrajectory<traj_t>::TrackStateProxy;
Expand Down Expand Up @@ -238,13 +243,17 @@ void addToGx2fSums(BoundMatrix& aMatrix, BoundVector& bVector, double& chi2sum,
const BoundMatrix& jacobianFromStart,
const track_state_t& trackState, const Logger& logger) {
BoundVector predicted = trackState.predicted();

ActsVector<kMeasDim> measurement = trackState.template calibrated<kMeasDim>();

ActsSquareMatrix<kMeasDim> covarianceMeasurement =
trackState.template calibratedCovariance<kMeasDim>();

ActsMatrix<kMeasDim, eBoundSize> projector =
trackState.projector().template topLeftCorner<kMeasDim, eBoundSize>();

ActsMatrix<kMeasDim, eBoundSize> projJacobian = projector * jacobianFromStart;

ActsMatrix<kMeasDim, 1> projPredicted = projector * predicted;

ActsVector<kMeasDim> residual = measurement - projPredicted;
Expand Down Expand Up @@ -440,41 +449,32 @@ class Gx2Fitter {
auto surface = navigator.currentSurface(state.navigation);
if (surface != nullptr) {
++result.surfaceCount;
ACTS_VERBOSE("Surface " << surface->geometryId() << " detected.");
const GeometryIdentifier geoId = surface->geometryId();
ACTS_DEBUG("Surface " << geoId << " detected.");

// Found material
if (surface->surfaceMaterial() != nullptr) {
ACTS_DEBUG(" The surface contains material.");
}

// Check if we have a measurement surface
if (auto sourcelink_it = inputMeasurements->find(surface->geometryId());
if (auto sourcelink_it = inputMeasurements->find(geoId);
sourcelink_it != inputMeasurements->end()) {
ACTS_VERBOSE("Measurement surface " << surface->geometryId()
<< " detected.");
ACTS_DEBUG(" The surface contains a measurement.");

// Transport the covariance to the surface
stepper.transportCovarianceToBound(state.stepping, *surface,
freeToBoundCorrection);

ACTS_VERBOSE(
"Actor - indices before processing:"
<< "\n "
<< "result.lastMeasurementIndex: " << result.lastMeasurementIndex
<< "\n "
<< "result.lastTrackIndex: " << result.lastTrackIndex << "\n "
<< "result.fittedStates->size(): " << result.fittedStates->size())

// TODO generalize the update of the currentTrackIndex
auto& fittedStates = *result.fittedStates;

// Mask for the track states. We don't need Smoothed and Filtered
TrackStatePropMask mask = TrackStatePropMask::Predicted |
TrackStatePropMask::Jacobian |
TrackStatePropMask::Calibrated;

ACTS_VERBOSE(" processSurface: addTrackState");

// Add a <mask> TrackState entry multi trajectory. This allocates
// storage for all components, which we will set later.
// Add a <trackStateMask> TrackState entry multi trajectory. This
// allocates storage for all components, which we will set later.
typename traj_t::TrackStateProxy trackStateProxy =
fittedStates.makeTrackState(mask, result.lastTrackIndex);
std::size_t currentTrackIndex = trackStateProxy.index();
fittedStates.makeTrackState(Gx2fConstants::trackStateMask,
result.lastTrackIndex);
const std::size_t currentTrackIndex = trackStateProxy.index();

// Set the trackStateProxy components with the state from the ongoing
// propagation
Expand Down Expand Up @@ -513,15 +513,7 @@ class Gx2Fitter {
typeFlags.set(TrackStateFlag::MeasurementFlag);
// We count the processed measurement
++result.processedMeasurements;
ACTS_VERBOSE("Actor - indices after processing, before over writing:"
<< "\n "
<< "result.lastMeasurementIndex: "
<< result.lastMeasurementIndex << "\n "
<< "trackStateProxy.index(): " << trackStateProxy.index()
<< "\n "
<< "result.lastTrackIndex: " << result.lastTrackIndex
<< "\n "
<< "currentTrackIndex: " << currentTrackIndex)

result.lastMeasurementIndex = currentTrackIndex;
result.lastTrackIndex = currentTrackIndex;

Expand All @@ -543,27 +535,19 @@ class Gx2Fitter {
<< " detected.");

// We only create track states here if there is already a measurement
// detected or if the surface has material (no holes before the first
// measurement)
if (result.measurementStates > 0
// || surface->surfaceMaterial() != nullptr
) {
ACTS_VERBOSE("Handle hole.");
// detected (no holes before the first measurement)
if (result.measurementStates > 0) {
ACTS_DEBUG(" Handle hole.");

auto& fittedStates = *result.fittedStates;

// Mask for the track states. We don't need Smoothed and Filtered
TrackStatePropMask mask = TrackStatePropMask::Predicted |
TrackStatePropMask::Jacobian |
TrackStatePropMask::Calibrated;

ACTS_VERBOSE(" processSurface: addTrackState");

// Add a <mask> TrackState entry multi trajectory. This allocates
// storage for all components, which we will set later.
// Add a <trackStateMask> TrackState entry multi trajectory. This
// allocates storage for all components, which we will set later.
typename traj_t::TrackStateProxy trackStateProxy =
fittedStates.makeTrackState(mask, result.lastTrackIndex);
std::size_t currentTrackIndex = trackStateProxy.index();
fittedStates.makeTrackState(Gx2fConstants::trackStateMask,
result.lastTrackIndex);
const std::size_t currentTrackIndex = trackStateProxy.index();

{
// Set the trackStateProxy components with the state from the
// ongoing propagation
Expand Down Expand Up @@ -604,16 +588,6 @@ class Gx2Fitter {
}
}

ACTS_VERBOSE(
"Actor - indices after processing, before over writing:"
<< "\n "
<< "result.lastMeasurementIndex: "
<< result.lastMeasurementIndex << "\n "
<< "trackStateProxy.index(): " << trackStateProxy.index()
<< "\n "
<< "result.lastTrackIndex: " << result.lastTrackIndex
<< "\n "
<< "currentTrackIndex: " << currentTrackIndex)
result.lastTrackIndex = currentTrackIndex;

if (trackStateProxy.typeFlags().test(TrackStateFlag::HoleFlag)) {
Expand All @@ -623,7 +597,7 @@ class Gx2Fitter {

++result.processedStates;
} else {
ACTS_VERBOSE("Ignoring hole, because no preceding measurements.");
ACTS_DEBUG(" Ignoring hole, because no preceding measurements.");
}

if (surface->surfaceMaterial() != nullptr) {
Expand All @@ -633,12 +607,12 @@ class Gx2Fitter {
// MaterialUpdateStage::FullUpdate);
}
} else {
ACTS_INFO("Actor: This case is not implemented yet")
ACTS_INFO("Surface " << geoId << " has no measurement/material/hole.")
}
}
ACTS_DEBUG("result.processedMeasurements: "
<< result.processedMeasurements << "\n"
<< "inputMeasurements.size(): " << inputMeasurements->size())
ACTS_VERBOSE("result.processedMeasurements: "
<< result.processedMeasurements << "\n"
<< "inputMeasurements.size(): " << inputMeasurements->size())
if (result.processedMeasurements >= inputMeasurements->size()) {
ACTS_INFO("Actor: finish: all measurements found.");
result.finished = true;
Expand Down Expand Up @@ -700,7 +674,7 @@ class Gx2Fitter {
const -> std::enable_if_t<
!_isdn, Result<typename TrackContainer<track_container_t, traj_t,
holder_t>::TrackProxy>> {
// Preprocess Measurements (Sourcelinks -> map)
// Preprocess Measurements (SourceLinks -> map)
// To be able to find measurements later, we put them into a map
// We need to copy input SourceLinks anyway, so the map can own them.
ACTS_VERBOSE("Preparing " << std::distance(it, end)
Expand Down Expand Up @@ -793,6 +767,11 @@ class Gx2Fitter {
propagationResult,
propagatorOptions, false);

if (!result.ok()) {
ACTS_ERROR("Propagation failed: " << result.error());
return result.error();
}

// TODO Improve Propagator + Actor [allocate before loop], rewrite
// makeMeasurements
auto& propRes = *result;
Expand Down
Loading

0 comments on commit e23732e

Please sign in to comment.