Skip to content

Commit

Permalink
refactor: Optimize TrapezoidBounds inside check (#1118)
Browse files Browse the repository at this point in the history
This was previously delegating to `BoundaryCheck::isInside` with a vector of vertices. This PR:

- Does a number of early checks for the trapezoid shape: inside core rectangle, outside y, outside larger x halflength
- Uses an `std::array` for the vertices it passes to the `isInside` call, which reduces the number of heap allocations.
  • Loading branch information
paulgessinger committed Jan 24, 2022
1 parent b6695fd commit a0e4a18
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 6 deletions.
33 changes: 32 additions & 1 deletion Core/src/Surfaces/TrapezoidBounds.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,38 @@ Acts::SurfaceBounds::BoundsType Acts::TrapezoidBounds::type() const {

bool Acts::TrapezoidBounds::inside(const Acts::Vector2& lposition,
const Acts::BoundaryCheck& bcheck) const {
return bcheck.isInside(lposition, vertices());
const double x = lposition[0];
const double y = lposition[1];

const double hlY = get(TrapezoidBounds::eHalfLengthY);
const double hlXnY = get(TrapezoidBounds::eHalfLengthXnegY);
const double hlXpY = get(TrapezoidBounds::eHalfLengthXposY);

if (bcheck.type() == BoundaryCheck::Type::eAbsolute) {
double tolX = bcheck.tolerance()[eBoundLoc0];
double tolY = bcheck.tolerance()[eBoundLoc1];

if (std::abs(y) - hlY > tolY) {
// outside y range
return false;
}

if (std::abs(x) - std::max(hlXnY, hlXpY) > tolX) {
// outside x range
return false;
}

if (std::abs(x) - std::min(hlXnY, hlXpY) <= tolX) {
// inside x range
return true;
}
}

// at this stage, the point can only be in the triangles
// run slow-ish polygon check
std::array<Vector2, 4> v{
Vector2{-hlXnY, -hlY}, {hlXnY, -hlY}, {hlXpY, hlY}, {-hlXpY, hlY}};
return bcheck.isInside(lposition, v);
}

std::vector<Acts::Vector2> Acts::TrapezoidBounds::vertices(
Expand Down
79 changes: 74 additions & 5 deletions Tests/UnitTests/Core/Surfaces/TrapezoidBoundsTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

#include <limits>

namespace utf = boost::unit_test;
namespace bdata = boost::unit_test::data;

namespace Acts {

Expand Down Expand Up @@ -74,7 +74,7 @@ BOOST_AUTO_TEST_CASE(TrapezoidBoundsException) {
}

/// Unit tests for TrapezoidBounds properties
BOOST_AUTO_TEST_CASE(TrapezoidBoundsProperties, *utf::expected_failures(3)) {
BOOST_AUTO_TEST_CASE(TrapezoidBoundsProperties) {
double minHalfX(1.), maxHalfX(6.), halfY(2.);
//
TrapezoidBounds trapezoidBoundsObject(minHalfX, maxHalfX, halfY);
Expand All @@ -100,7 +100,7 @@ BOOST_AUTO_TEST_CASE(TrapezoidBoundsProperties, *utf::expected_failures(3)) {

/// Test vertices
std::vector<Vector2> expectedVertices{
{1., -2.}, {6., 2.}, {-6., 2.}, {-1., -2.}};
{-1., -2.}, {1., -2.}, {6., 2.}, {-6., 2.}};
const auto& actualVertices = trapezoidBoundsObject.vertices();
BOOST_CHECK_EQUAL_COLLECTIONS(actualVertices.cbegin(), actualVertices.cend(),
expectedVertices.cbegin(),
Expand All @@ -120,13 +120,82 @@ BOOST_AUTO_TEST_CASE(TrapezoidBoundsProperties, *utf::expected_failures(3)) {
boost::test_tools::output_test_stream dumpOuput;
trapezoidBoundsObject.toStream(dumpOuput);
BOOST_CHECK(
dumpOuput.is_equal("Acts::TrapezoidBounds: (minHlengthX, maxHlengthX, "
"hlengthY) = (1.0000000, 6.0000000, 2.0000000)"));
dumpOuput.is_equal("Acts::TrapezoidBounds: (halfXnegY, halfXposY, "
"halfY) = (1.0000000, 6.0000000, 2.0000000)"));
//
/// Test inside
BOOST_CHECK(trapezoidBoundsObject.inside(inRectangle, BoundaryCheck(true)));
BOOST_CHECK(!trapezoidBoundsObject.inside(outside, BoundaryCheck(true)));

const auto vertices = trapezoidBoundsObject.vertices();
BoundaryCheck bc{true};

std::vector<Vector2> testPoints = {
// inside
{0, 1},
{0, -1},
{2, 0.5},
{2, 0},
{-2, 0},
{-2, 0.5},
{3, 1},
{-3, 1},
{4, 1},
{-4, 1},
{-6, 2},

// outside
{0, 2.5},
{0, -2.5},
{2, 2.5},
{-2, 2.5},
{2, -2.5},
{2, -2.5},
{4, -1},
{-4, -1},
{-7, 0},
{7, 0},
{5, -3},
{5, 3},
{-5, -3},
{-5, 3},
{6, 2},

};

for (const auto& p : testPoints) {
BOOST_TEST_CONTEXT("p=" << p.transpose()) {
BOOST_CHECK_EQUAL(bc.isInside(p, vertices),
trapezoidBoundsObject.inside(p, bc));
}
}
}

BOOST_DATA_TEST_CASE(
TrapezoidInsideCheck,
bdata::random((bdata::seed = 1,
bdata::distribution = std::uniform_real_distribution<>(-7,
7))) ^
bdata::random(
(bdata::seed = 2,
bdata::distribution = std::uniform_real_distribution<>(-3, 3))) ^
bdata::xrange(1000) * bdata::make({0.0, 0.1, 0.2, 0.3}),
x, y, index, tol) {
(void)index;
double minHalfX(1.), maxHalfX(6.), halfY(2.);
static const TrapezoidBounds trapezoidBoundsObject(minHalfX, maxHalfX, halfY);
static const auto vertices = trapezoidBoundsObject.vertices();

BoundaryCheck bc{true};

if (tol != 0.0) {
bc = BoundaryCheck{true, true, tol, tol};
}

BOOST_CHECK_EQUAL(bc.isInside({x, y}, vertices),
trapezoidBoundsObject.inside({x, y}, bc));
}

/// Unit test for testing TrapezoidBounds assignment
BOOST_AUTO_TEST_CASE(TrapezoidBoundsAssignment) {
double minHalfX(1.), maxHalfX(6.), halfY(2.);
Expand Down

0 comments on commit a0e4a18

Please sign in to comment.