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!: Introduce regular surface intermediate class #2340

Merged
merged 33 commits into from
Nov 11, 2023
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
dbcc374
step1 : introduce intermediate class for normal methods
paulgessinger Jul 27, 2023
70c78fa
fix line surface unit tests
paulgessinger Jul 27, 2023
2a726fd
wip
paulgessinger Jul 28, 2023
8c677e3
Add coerceToSurface, use it explicitly
paulgessinger Aug 1, 2023
9486294
auto-coerce in pathCorrection
paulgessinger Aug 1, 2023
86123eb
RegularSurface does not have a no-position normal overload anymore
paulgessinger Aug 1, 2023
fa181b6
bump propagator / navigator verbosity in tests
paulgessinger Aug 1, 2023
ad406db
add overloads to RegularSurface without direction for globalToLocal
paulgessinger Aug 1, 2023
5b1ea14
fix: Fix surface friend declaration for Doxygen
paulgessinger Aug 1, 2023
4dc5bae
fix ACTSVG normal usage
paulgessinger Aug 1, 2023
bb1c20e
brute force unused variable
paulgessinger Aug 1, 2023
8809aef
add localToGlobal and coerceToSurface in RegularSurface
paulgessinger Aug 2, 2023
236a79b
coerce to surface in boundary surface check
paulgessinger Aug 2, 2023
14fa9c3
fix missing auto-fpe-masks?
paulgessinger Aug 2, 2023
bf1b7c2
docs fixes
paulgessinger Aug 2, 2023
1794011
spelling
paulgessinger Aug 2, 2023
a474a43
revert: Changes to normal position preconditions
paulgessinger Aug 3, 2023
600d462
update fpe location
paulgessinger Aug 4, 2023
14b6b5b
Merge branch 'main' into normal-imprv-only
asalzburger Aug 8, 2023
574c45c
Merge remote-tracking branch 'origin/main' into normal-imprv-only
paulgessinger Aug 29, 2023
d4f5cd6
pr feedback
paulgessinger Aug 29, 2023
44a3a50
Merge remote-tracking branch 'origin/main' into normal-imprv-only
paulgessinger Sep 18, 2023
d95fd32
Update CI/physmon/fpe_masks.yml
paulgessinger Sep 18, 2023
d093476
Merge remote-tracking branch 'origin/main' into normal-imprv-only
paulgessinger Sep 20, 2023
4635305
Merge branch 'main' into normal-imprv-only
paulgessinger Sep 21, 2023
c46104b
Merge remote-tracking branch 'origin/main' into normal-imprv-only
paulgessinger Sep 25, 2023
ca1f7bd
tiny compilation fix
paulgessinger Sep 25, 2023
f120883
Merge branch 'main' into normal-imprv-only
kodiakhq[bot] Sep 25, 2023
f08e295
Merge remote-tracking branch 'origin/main' into normal-imprv-only
paulgessinger Nov 8, 2023
c5da2d7
add doc
paulgessinger Nov 8, 2023
66767c3
fix surface stub
paulgessinger Nov 9, 2023
af6ce79
doc fix
paulgessinger Nov 9, 2023
d2f6c21
Merge branch 'main' into normal-imprv-only
kodiakhq[bot] Nov 11, 2023
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
12 changes: 7 additions & 5 deletions Core/include/Acts/Detector/Portal.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "Acts/Navigation/NavigationDelegates.hpp"
#include "Acts/Navigation/NavigationState.hpp"
#include "Acts/Surfaces/BoundaryCheck.hpp"
#include "Acts/Surfaces/RegularSurface.hpp"
#include "Acts/Surfaces/Surface.hpp"

#include <array>
Expand Down Expand Up @@ -45,7 +46,7 @@ class Portal : public std::enable_shared_from_this<Portal> {
/// Constructor from surface w/o portal links
///
/// @param surface is the representing surface
Portal(std::shared_ptr<Surface> surface);
Portal(std::shared_ptr<RegularSurface> surface);

public:
/// The volume links forward/backward with respect to the surface normal
Expand All @@ -60,7 +61,8 @@ class Portal : public std::enable_shared_from_this<Portal> {
friend class DetectorVolume;

/// Factory for producing memory managed instances of Portal.
static std::shared_ptr<Portal> makeShared(std::shared_ptr<Surface> surface);
static std::shared_ptr<Portal> makeShared(
std::shared_ptr<RegularSurface> surface);

/// Retrieve a @c std::shared_ptr for this surface (non-const version)
///
Expand Down Expand Up @@ -88,10 +90,10 @@ class Portal : public std::enable_shared_from_this<Portal> {
virtual ~Portal() = default;

/// Const access to the surface representation
const Surface& surface() const;
const RegularSurface& surface() const;

/// Non-const access to the surface reference
Surface& surface();
RegularSurface& surface();

/// Update the current volume
///
Expand Down Expand Up @@ -147,7 +149,7 @@ class Portal : public std::enable_shared_from_this<Portal> {

private:
/// The surface representation of this portal
std::shared_ptr<Surface> m_surface;
std::shared_ptr<RegularSurface> m_surface;

/// The portal targets along/opposite the normal vector
DetectorVolumeUpdators m_volumeUpdators = {unconnectedUpdator(),
Expand Down
3 changes: 2 additions & 1 deletion Core/include/Acts/Digitization/DigitizationModule.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "Acts/Definitions/Algebra.hpp"
#include "Acts/Digitization/DigitizationCell.hpp"
#include "Acts/Digitization/Segmentation.hpp"
#include "Acts/Surfaces/RegularSurface.hpp"

#include <memory>
#include <vector>
Expand All @@ -18,7 +19,7 @@ namespace Acts {

class Surface;

using SurfacePtr = std::shared_ptr<const Surface>;
using SurfacePtr = std::shared_ptr<const RegularSurface>;
using SurfacePtrVector = std::vector<SurfacePtr>;

/// @class DigitizationModule
Expand Down
3 changes: 2 additions & 1 deletion Core/include/Acts/Digitization/Segmentation.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#pragma once
#include "Acts/Definitions/Algebra.hpp"
#include "Acts/Digitization/DigitizationCell.hpp"
#include "Acts/Surfaces/RegularSurface.hpp"

#include <memory>
#include <vector>
Expand All @@ -18,7 +19,7 @@ namespace Acts {
class SurfaceBounds;
class Surface;
class BinUtility;
using SurfacePtr = std::shared_ptr<const Surface>;
using SurfacePtr = std::shared_ptr<const RegularSurface>;
using SurfacePtrVector = std::vector<SurfacePtr>;

/// @brief Segmentation Base class
Expand Down
15 changes: 8 additions & 7 deletions Core/include/Acts/Geometry/BoundarySurfaceT.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "Acts/Geometry/BoundarySurfaceFace.hpp"
#include "Acts/Geometry/GeometryContext.hpp"
#include "Acts/Geometry/Volume.hpp"
#include "Acts/Surfaces/RegularSurface.hpp"
#include "Acts/Surfaces/Surface.hpp"
#include "Acts/Utilities/BinnedArray.hpp"

Expand Down Expand Up @@ -57,7 +58,7 @@ class BoundarySurfaceT {
/// @param surface The unique surface the boundary represents
/// @param inside The inside volume the boundary surface points to
/// @param outside The outside volume the boundary surface points to
BoundarySurfaceT(std::shared_ptr<const Surface> surface,
BoundarySurfaceT(std::shared_ptr<const RegularSurface> surface,
const volume_t* inside, const volume_t* outside)
: m_surface(std::move(surface)),
m_oppositeVolume(inside),
Expand All @@ -71,8 +72,8 @@ class BoundarySurfaceT {
/// @param surface The unique surface the boundary represents
/// @param inside The inside volume the boundary surface points to
/// @param outside The outside volume the boundary surface points to
BoundarySurfaceT(std::shared_ptr<const Surface> surface, VolumePtr inside,
VolumePtr outside)
BoundarySurfaceT(std::shared_ptr<const RegularSurface> surface,
VolumePtr inside, VolumePtr outside)
: m_surface(std::move(surface)),
m_oppositeVolume(inside.get()),
m_alongVolume(outside.get()),
Expand All @@ -86,7 +87,7 @@ class BoundarySurfaceT {
/// @param insideArray The inside volume array the boundary surface points to
/// @param outsideArray The outside volume array the boundary surface
/// points to
BoundarySurfaceT(std::shared_ptr<const Surface> surface,
BoundarySurfaceT(std::shared_ptr<const RegularSurface> surface,
std::shared_ptr<const VolumeArray> insideArray,
std::shared_ptr<const VolumeArray> outsideArray)
: m_surface(std::move(surface)),
Expand Down Expand Up @@ -122,7 +123,7 @@ class BoundarySurfaceT {
}

/// The Surface Representation of this
virtual const Surface& surfaceRepresentation() const;
virtual const RegularSurface& surfaceRepresentation() const;

/// Helper method: attach a Volume to this BoundarySurfaceT
/// this is done during the geometry construction.
Expand All @@ -141,7 +142,7 @@ class BoundarySurfaceT {

protected:
/// the represented surface by this
std::shared_ptr<const Surface> m_surface;
std::shared_ptr<const RegularSurface> m_surface;
/// the inside (w.r.t. normal vector) volume to point to if only one exists
const volume_t* m_oppositeVolume;
/// the outside (w.r.t. normal vector) volume to point to if only one exists
Expand All @@ -153,7 +154,7 @@ class BoundarySurfaceT {
};

template <class volume_t>
inline const Surface& BoundarySurfaceT<volume_t>::surfaceRepresentation()
inline const RegularSurface& BoundarySurfaceT<volume_t>::surfaceRepresentation()
const {
return (*(m_surface.get()));
}
Expand Down
3 changes: 2 additions & 1 deletion Core/include/Acts/Geometry/VolumeBounds.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "Acts/Definitions/Algebra.hpp"
#include "Acts/Definitions/Direction.hpp"
#include "Acts/Geometry/Volume.hpp"
#include "Acts/Surfaces/RegularSurface.hpp"
#include "Acts/Utilities/BinningType.hpp"

#include <cmath>
Expand All @@ -27,7 +28,7 @@ class Direction;

using VolumeBoundsPtr = std::shared_ptr<const VolumeBounds>;

using OrientedSurface = std::pair<std::shared_ptr<Surface>, Direction>;
using OrientedSurface = std::pair<std::shared_ptr<RegularSurface>, Direction>;
using OrientedSurfaces = std::vector<OrientedSurface>;

// Planar definitions to help construct the boundary surfaces
Expand Down
22 changes: 10 additions & 12 deletions Core/include/Acts/Surfaces/ConeSurface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "Acts/Geometry/Polyhedron.hpp"
#include "Acts/Surfaces/BoundaryCheck.hpp"
#include "Acts/Surfaces/ConeBounds.hpp"
#include "Acts/Surfaces/RegularSurface.hpp"
#include "Acts/Surfaces/Surface.hpp"
#include "Acts/Utilities/BinningType.hpp"
#include "Acts/Utilities/Result.hpp"
Expand All @@ -39,10 +40,8 @@ namespace Acts {
/// Propagations to a cone surface will be returned in
/// curvilinear coordinates.

class ConeSurface : public Surface {
#ifndef DOXYGEN
friend Surface;
#endif
class ConeSurface : public RegularSurface {
friend class Surface;

protected:
/// Constructor form HepTransform and an opening angle
Expand Down Expand Up @@ -134,9 +133,6 @@ class ConeSurface : public Surface {
Vector3 normal(const GeometryContext& gctx,
const Vector3& position) const final;

/// Normal vector return without argument
using Surface::normal;

// Return method for the rotational symmetry axis
///
/// @param gctx The current geometry context object, e.g. alignment
Expand All @@ -151,24 +147,26 @@ class ConeSurface : public Surface {
///
/// @param gctx The current geometry context object, e.g. alignment
/// @param lposition is the local position to be transformed
/// @param direction is the global momentum direction (ignored in this operation)
///
/// @return The global position by value
Vector3 localToGlobal(const GeometryContext& gctx, const Vector2& lposition,
const Vector3& direction) const final;
Vector3 localToGlobal(const GeometryContext& gctx,
const Vector2& lposition) const final;

// Use overloads from `RegularSurface`
using RegularSurface::globalToLocal;
using RegularSurface::localToGlobal;
using RegularSurface::normal;

/// Global to local transformation
///
/// @param gctx The current geometry context object, e.g. alignment
/// @param position is the global position to be transformed
/// @param direction is the global momentum direction (ignored in this operation)
/// @param tolerance optional tolerance within which a point is considered
/// valid on surface
///
/// @return a Result<Vector2> which can be !ok() if the operation fails
Result<Vector2> globalToLocal(
const GeometryContext& gctx, const Vector3& position,
const Vector3& direction,
double tolerance = s_onSurfaceTolerance) const final;

/// Straight line intersection schema from position/direction
Expand Down
20 changes: 9 additions & 11 deletions Core/include/Acts/Surfaces/CylinderSurface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "Acts/Geometry/Polyhedron.hpp"
#include "Acts/Surfaces/BoundaryCheck.hpp"
#include "Acts/Surfaces/CylinderBounds.hpp"
#include "Acts/Surfaces/RegularSurface.hpp"
#include "Acts/Surfaces/Surface.hpp"
#include "Acts/Utilities/BinningType.hpp"
#include "Acts/Utilities/Result.hpp"
Expand All @@ -40,10 +41,8 @@ class DetectorElementBase;
///
/// @image html CylinderSurface.png

class CylinderSurface : public Surface {
#ifndef DOXYGEN
friend Surface;
#endif
class CylinderSurface : public RegularSurface {
friend class Surface;

protected:
/// Constructor from Transform3 and CylinderBounds
Expand Down Expand Up @@ -144,8 +143,10 @@ class CylinderSurface : public Surface {
Vector3 normal(const GeometryContext& gctx,
const Vector3& position) const final;

/// Normal vector return without argument
using Surface::normal;
// Use overloads from `RegularSurface`
using RegularSurface::globalToLocal;
using RegularSurface::localToGlobal;
using RegularSurface::normal;

/// Return method for the rotational symmetry axis
///
Expand All @@ -161,24 +162,21 @@ class CylinderSurface : public Surface {
///
/// @param gctx The current geometry context object, e.g. alignment
/// @param lposition is the local position to be transformed
/// @param direction is the global momentum direction (ignored in this operation)
///
/// @return The global position by value
Vector3 localToGlobal(const GeometryContext& gctx, const Vector2& lposition,
const Vector3& direction) const final;
Vector3 localToGlobal(const GeometryContext& gctx,
const Vector2& lposition) const final;

/// Global to local transformation
///
/// @param gctx The current geometry context object, e.g. alignment
/// @param position is the global position to be transformed
/// @param direction is the global momentum direction (ignored in this operation)
/// @param tolerance optional tolerance within which a point is considered
/// valid on surface
///
/// @return a Result<Vector2> which can be !ok() if the operation fails
Result<Vector2> globalToLocal(
const GeometryContext& gctx, const Vector3& position,
const Vector3& direction,
double tolerance = s_onSurfaceTolerance) const final;

/// Straight line intersection schema from position/direction
Expand Down
33 changes: 22 additions & 11 deletions Core/include/Acts/Surfaces/DiscSurface.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "Acts/Surfaces/BoundaryCheck.hpp"
#include "Acts/Surfaces/DiscBounds.hpp"
#include "Acts/Surfaces/InfiniteBounds.hpp"
#include "Acts/Surfaces/RegularSurface.hpp"
#include "Acts/Surfaces/Surface.hpp"
#include "Acts/Utilities/BinningType.hpp"
#include "Acts/Utilities/Result.hpp"
Expand Down Expand Up @@ -50,10 +51,8 @@ class SurfaceBounds;
///
/// @image html DiscSurface.png
///
class DiscSurface : public Surface {
#ifndef DOXYGEN
friend Surface;
#endif
class DiscSurface : public RegularSurface {
friend class Surface;

protected:
/// Constructor for Discs from Transform3, \f$ r_{min}, r_{max} \f$
Expand Down Expand Up @@ -119,6 +118,11 @@ class DiscSurface : public Surface {
/// Return the surface type
SurfaceType type() const override;

// User overloads from `RegularSurface`
using RegularSurface::globalToLocal;
using RegularSurface::localToGlobal;
using RegularSurface::normal;

/// Normal vector return
///
/// @param gctx The current geometry context object, e.g. alignment
Expand All @@ -128,8 +132,18 @@ class DiscSurface : public Surface {
Vector3 normal(const GeometryContext& gctx,
const Vector2& lposition) const final;

/// Normal vector return without argument
using Surface::normal;
/// Get the normal vector of this surface at a given global position
/// @note The @p position is required to be on-surface.
/// @param gctx The current geometry context object, e.g. alignment
/// @param position is the global positiono (for @ref DiscSurface this is ignored)
/// @return The normal vector
Vector3 normal(const GeometryContext& gctx,
const Vector3& position) const final;

/// Get the normal vector, independent of the location
/// @param gctx The current geometry context object, e.g. alignment
/// @return The normal vector
Vector3 normal(const GeometryContext& gctx) const;

/// The binning position The position calculated
/// for a certain binning type
Expand All @@ -150,26 +164,23 @@ class DiscSurface : public Surface {
///
/// @param gctx The current geometry context object, e.g. alignment
/// @param lposition local 2D position in specialized surface frame
/// @param direction global 3D momentum direction (optionally ignored)
///
/// @return global position by value
Vector3 localToGlobal(const GeometryContext& gctx, const Vector2& lposition,
const Vector3& direction) const final;
Vector3 localToGlobal(const GeometryContext& gctx,
const Vector2& lposition) const final;

/// Global to local transformation
/// @note the direction is ignored for Disc surfaces in this calculateion
///
/// @param gctx The current geometry context object, e.g. alignment
/// @param position global 3D position - considered to be on surface but not
/// inside bounds (check is done)
/// @param direction global 3D momentum direction (optionally ignored)
/// @param tolerance optional tolerance within which a point is considered
/// valid on surface
///
/// @return a Result<Vector2> which can be !ok() if the operation fails
Result<Vector2> globalToLocal(
const GeometryContext& gctx, const Vector3& position,
const Vector3& direction,
double tolerance = s_onSurfaceTolerance) const final;

/// Special method for DiscSurface : local<->local transformations polar <->
Expand Down
Loading
Loading