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

DM-9938: Make some afw types hashable #17

Merged
merged 2 commits into from
Nov 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ doc/doxygen.conf
examples/transform
tests/.tests
tests/.cache
tests/test_angle
tests/test_box
tests/test_polynomials
tests/test_coordinates
tests/test_spherePoint
Expand Down
22 changes: 22 additions & 0 deletions include/lsst/geom/Angle.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ class AngleUnit final {
*/
constexpr bool operator==(AngleUnit const& rhs) const noexcept;

/// Return a hash of this object.
std::size_t hash_value() const noexcept { return std::hash<double>()(_val); }

private:
double _val;
};
Expand Down Expand Up @@ -271,6 +274,9 @@ class Angle final {

#undef ANGLE_COMP

/// Return a hash of this object.
std::size_t hash_value() const noexcept { return std::hash<double>()(_val); }

private:
double _val;
};
Expand Down Expand Up @@ -445,4 +451,20 @@ inline Angle Angle::separation(Angle const& other) const noexcept { return (*thi
} // namespace geom
} // namespace lsst

namespace std {
template <>
struct hash<lsst::geom::AngleUnit> {
using argument_type = lsst::geom::AngleUnit;
using result_type = size_t;
size_t operator()(argument_type const& x) const noexcept { return x.hash_value(); }
};

template <>
struct hash<lsst::geom::Angle> {
using argument_type = lsst::geom::Angle;
using result_type = size_t;
size_t operator()(argument_type const& x) const noexcept { return x.hash_value(); }
};
} // namespace std

#endif // if !defined(LSST_GEOM_ANGLE_H)
22 changes: 22 additions & 0 deletions include/lsst/geom/Box.h
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,9 @@ class Box2I final {
*/
bool operator!=(Box2I const& other) const noexcept;

/// Return a hash of this object.
std::size_t hash_value() const noexcept;

/**
* Get the corner points
*
Expand Down Expand Up @@ -492,6 +495,9 @@ class Box2D final {
*/
bool operator!=(Box2D const& other) const noexcept;

/// Return a hash of this object.
std::size_t hash_value() const noexcept;

/**
* Get the corner points
*
Expand Down Expand Up @@ -528,4 +534,20 @@ std::ostream& operator<<(std::ostream& os, Box2D const& box);
} // namespace geom
} // namespace lsst

namespace std {
template <>
struct hash<lsst::geom::Box2I> {
using argument_type = lsst::geom::Box2I;
using result_type = size_t;
size_t operator()(argument_type const& x) const noexcept { return x.hash_value(); }
};

template <>
struct hash<lsst::geom::Box2D> {
using argument_type = lsst::geom::Box2D;
using result_type = size_t;
size_t operator()(argument_type const& x) const noexcept { return x.hash_value(); }
};
} // namespace std

#endif
13 changes: 13 additions & 0 deletions include/lsst/geom/Extent.h
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,10 @@ Extent<T, 3>::Extent(Point<U, 3> const &other) noexcept(IS_NOTHROW_CONVERTIBLE<T
this->setZ(static_cast<T>(other.getZ()));
};

// Hash functions
template <typename T, int N>
std::size_t hash_value(Extent<T, N> const &extent) noexcept;

typedef Extent<int, 2> ExtentI;
typedef Extent<int, 2> Extent2I;
typedef Extent<int, 3> Extent3I;
Expand Down Expand Up @@ -490,4 +494,13 @@ Extent<double, N> operator-(Extent<int, N> const &lhs, Extent<double, N> const &
} // namespace geom
} // namespace lsst

namespace std {
template <typename T, int N>
struct hash<lsst::geom::Extent<T, N>> {
using argument_type = lsst::geom::Extent<T, N>;
using result_type = std::size_t;
result_type operator()(argument_type const &x) const noexcept { return lsst::geom::hash_value(x); }
};
} // namespace std

#endif
11 changes: 10 additions & 1 deletion include/lsst/geom/Point.h
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ class Point<T, 3> : public PointBase<T, 3> {

// Hash functions
template <typename T, int N>
std::size_t hash_value(Point<T, N> const &point);
std::size_t hash_value(Point<T, N> const &point) noexcept;

typedef Point<int, 2> PointI;
typedef Point<int, 2> Point2I;
Expand Down Expand Up @@ -372,4 +372,13 @@ Extent<double, N> operator-(Point<int, N> const &lhs, Point<double, N> const &rh
} // namespace geom
} // namespace lsst

namespace std {
template <typename T, int N>
struct hash<lsst::geom::Point<T, N>> {
using argument_type = lsst::geom::Point<T, N>;
using result_type = std::size_t;
result_type operator()(argument_type const &x) const noexcept { return lsst::geom::hash_value(x); }
};
} // namespace std

#endif
12 changes: 12 additions & 0 deletions include/lsst/geom/SpherePoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,9 @@ class SpherePoint final {
*/
bool operator!=(SpherePoint const& other) const noexcept;

/// Return a hash of this object.
std::size_t hash_value() const noexcept;

/**
* Orientation at this point of the great circle arc to another point.
*
Expand Down Expand Up @@ -408,4 +411,13 @@ std::ostream& operator<<(std::ostream& os, SpherePoint const& point);
} // namespace geom
} // namespace lsst

namespace std {
template <>
struct hash<lsst::geom::SpherePoint> {
using argument_type = lsst::geom::SpherePoint;
using result_type = size_t;
size_t operator()(argument_type const& x) const noexcept { return x.hash_value(); }
};
} // namespace std

#endif /* LSST_GEOM_SPHEREPOINT_H_ */
16 changes: 16 additions & 0 deletions src/Box.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <cmath>
#include <limits>

#include "lsst/utils/hashCombine.h"
#include "lsst/geom/Box.h"

namespace lsst {
Expand Down Expand Up @@ -218,6 +219,11 @@ bool Box2I::operator!=(Box2I const& other) const noexcept {
return other._minimum != this->_minimum || other._dimensions != this->_dimensions;
}

std::size_t Box2I::hash_value() const noexcept {
// Completely arbitrary seed
return utils::hashCombine(17, _minimum, _dimensions);
}

std::vector<Point2I> Box2I::getCorners() const {
std::vector<Point2I> retVec;
retVec.push_back(getMin());
Expand Down Expand Up @@ -398,6 +404,16 @@ bool Box2D::operator!=(Box2D const& other) const noexcept {
(other._minimum != this->_minimum || other._maximum != this->_maximum);
}

std::size_t Box2D::hash_value() const noexcept {
if (isEmpty()) {
// All empty boxes are equal and must have equal hashes
return 179;
} else {
// Completely arbitrary seed
return utils::hashCombine(17, _minimum, _maximum);
}
Copy link
Member

Choose a reason for hiding this comment

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

I suspect the magic numbers here are completely arbitrary and just need to be nonzero and deterministic, but even if that's the case I think a code comment would be welcome. That's especially true if they have other useful properties, like being odd numbers.

Copy link
Member Author

@kfindeisen kfindeisen Nov 6, 2018

Choose a reason for hiding this comment

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

I feel better if they're prime numbers (though I do violate that rule with 42 😉), but yes, they're basically arbitrary. I'll go ahead and add a comment, but I feel like there's too much boilerplate already.

P.S. Actually, the seed need not be deterministic, as long as you only generate it once -- that's how you get salted hashes.

}

std::vector<Point2D> Box2D::getCorners() const {
std::vector<Point2D> retVec;
retVec.push_back(getMin());
Expand Down
13 changes: 13 additions & 0 deletions src/Extent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/

#include "lsst/utils/hashCombine.h"
#include "lsst/geom/CoordinateBase.h"
#include "lsst/geom/Point.h"
#include "lsst/geom/Extent.h"
Expand Down Expand Up @@ -122,6 +123,13 @@ Extent<int, N> ceil(Extent<double, N> const &input) noexcept {
return result;
}

template <typename T, int N>
std::size_t hash_value(Extent<T, N> const &extent) noexcept {
std::size_t result = 0; // Completely arbitrary seed
for (int n = 0; n < N; ++n) result = utils::hashCombine(result, extent[n]);
return result;
}

#ifndef DOXYGEN

template class ExtentBase<int, 2>;
Expand All @@ -144,6 +152,11 @@ template Extent<int, 3> floor(Extent<double, 3> const &);
template Extent<int, 2> ceil(Extent<double, 2> const &);
template Extent<int, 3> ceil(Extent<double, 3> const &);

template std::size_t hash_value(Extent<int, 2> const &extent) noexcept;
template std::size_t hash_value(Extent<int, 3> const &extent) noexcept;
template std::size_t hash_value(Extent<double, 2> const &extent) noexcept;
template std::size_t hash_value(Extent<double, 3> const &extent) noexcept;

#endif // !DOXYGEN

} // namespace geom
Expand Down
4 changes: 2 additions & 2 deletions src/Point.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ CoordinateExpr<N> PointBase<T, N>::ge(Point<T, N> const &other) const noexcept {
}

template <typename T, int N>
std::size_t hash_value(Point<T, N> const &point) {
std::size_t result = 0;
std::size_t hash_value(Point<T, N> const &point) noexcept {
std::size_t result = 0; // Completely arbitrary seed
for (int n = 0; n < N; ++n) result = utils::hashCombine(result, point[n]);
return result;
}
Expand Down
6 changes: 6 additions & 0 deletions src/SpherePoint.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#include "Eigen/Geometry"

#include "lsst/utils/hashCombine.h"
#include "lsst/geom/SpherePoint.h"
#include "lsst/geom/sphgeomUtils.h"
#include "lsst/pex/exceptions.h"
Expand Down Expand Up @@ -150,6 +151,11 @@ bool SpherePoint::operator==(SpherePoint const& other) const noexcept {

bool SpherePoint::operator!=(SpherePoint const& other) const noexcept { return !(*this == other); }

std::size_t SpherePoint::hash_value() const noexcept {
// Completely arbitrary seed
return utils::hashCombine(17, _longitude, _latitude);
}

Angle SpherePoint::bearingTo(SpherePoint const& other) const {
Angle const deltaLon = other.getLongitude() - this->getLongitude();

Expand Down
45 changes: 45 additions & 0 deletions tests/test_angle.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Developed for the LSST Data Management System.
* This product includes software developed by the LSST Project
* (https://www.lsst.org).
* See the COPYRIGHT file at the top-level directory of this distribution
* for details of code ownership.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/

#define BOOST_TEST_DYN_LINK
#define BOOST_TEST_MODULE AngleCpp
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wunused-variable"
#include "boost/test/unit_test.hpp"
#pragma clang diagnostic pop

#include "lsst/utils/tests.h"

#include "lsst/geom/Angle.h"

namespace lsst {
namespace geom {

BOOST_AUTO_TEST_CASE(Hash) {
utils::assertValidHash<Angle>();
utils::assertHashesEqual(0.0 * radians, 0.0 * degrees);

utils::assertValidHash<AngleUnit>();
utils::assertHashesEqual(degrees, AngleUnit(PI / 180.0));
}

} // namespace geom
} // namespace lsst
60 changes: 60 additions & 0 deletions tests/test_box.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* Developed for the LSST Data Management System.
* This product includes software developed by the LSST Project
* (https://www.lsst.org).
* See the COPYRIGHT file at the top-level directory of this distribution
* for details of code ownership.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/

#define BOOST_TEST_DYN_LINK
#define BOOST_TEST_MODULE BoxCpp

#include "boost/test/unit_test.hpp"

#include "lsst/utils/tests.h"

#include "lsst/geom/Box.h"

/*
* Unit tests for C++-only functionality in Box2I and Box2D.
*
* See test_box.py for remaining unit tests.
*/
namespace lsst {
namespace geom {

BOOST_AUTO_TEST_CASE(Box2IHash) {
using Int = Box2I::Element;
using Point = Box2I::Point;

utils::assertValidHash<Box2I>();
utils::assertHashesEqual(Box2I(Point(0, 0), Point(24, 200)), Box2I(Point(0, 0), Extent2I(25, 201)));
utils::assertHashesEqual(Box2I(), Box2I(Point(24, 200), Point(0, 0), false));
}

BOOST_AUTO_TEST_CASE(Box2DHash) {
using Real = Box2D::Element;
using Point = Box2D::Point;
static const Real nan = std::numeric_limits<Real>::quiet_NaN();

utils::assertValidHash<Box2D>();
utils::assertHashesEqual(Box2D(Point(0, 0), Point(24, 20.5)), Box2D(Point(0, 0), Extent2D(24, 20.5)));
utils::assertHashesEqual(Box2D(), Box2D(Point(24, 20.5), Point(0, 0), false));
utils::assertHashesEqual(Box2D(), Box2D(Point(nan), Point(42.0, 52.0)));
}

} // namespace geom
} // namespace lsst