Skip to content

Commit

Permalink
clean up code
Browse files Browse the repository at this point in the history
  • Loading branch information
Jonathan24680 committed Jan 17, 2025
1 parent d5c934c commit c1dc963
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 31 deletions.
2 changes: 1 addition & 1 deletion src/engine/SpatialJoinAlgorithms.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ std::vector<Box> SpatialJoinAlgorithms::computeBoundingBox(

// safety buffer for numerical inaccuracies
double maxDistInMetersBuffer = maxDist.value() + additionalDist;
if (maxDist.value() < 10) {
if (maxDistInMetersBuffer < 10) {
maxDistInMetersBuffer = 10;
} else if (static_cast<double>(maxDist.value()) <
static_cast<double>(std::numeric_limits<long long>::max()) /
Expand Down
4 changes: 1 addition & 3 deletions src/engine/SpatialJoinAlgorithms.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ namespace bgi = boost::geometry::index;

using Point = bg::model::point<double, 2, bg::cs::cartesian>;
using Box = bg::model::box<Point>;
// using Value = std::pair<Point, size_t>; // TODO: remove this line, it's
// legacy, when areas were not yet possible
using Value = std::pair<Box, size_t>;
using Polygon = boost::geometry::model::polygon<
boost::geometry::model::d2::point_xy<double>>;
Expand Down Expand Up @@ -102,7 +100,7 @@ class SpatialJoinAlgorithms {
// map and in the internal representation it looks like two/more boxes). The
// additionalDist gets added on the max distance to compensate for areas being
// bigger than points. AdditionalDist must be the max distance from the
// midpoint of the bounding box of the area to any point inside the method.
// midpoint of the bounding box of the area to any point inside the area.
// The function getMaxDistFromMidpointToAnyPointInsideTheBox() can be used to
// calculate it.
std::vector<BoostGeometryNamespace::Box> computeBoundingBox(
Expand Down
18 changes: 6 additions & 12 deletions test/engine/SpatialJoinAlgorithmsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ using SJ = std::variant<NearestNeighborsConfig, MaxDistanceConfig>;
namespace computeResultTest {

// Represents from left to right: the algorithm, addLeftChildFirst,
// bigChildLeft, a spatial join task and if areas or points should be used
// bigChildLeft, a spatial join task and if areas (=true) or points (=false)
// should be used
using SpatialJoinTestParam =
std::tuple<SpatialJoinAlgorithm, bool, bool, SpatialJoinTask, bool>;

Expand Down Expand Up @@ -277,7 +278,7 @@ class SpatialJoinParamTest

void testWrongPointInInput(SJ task, bool addLeftChildFirst,
Rows expectedOutput, Row columnNames) {
auto kg = createSmallDatasetWithPoints();
auto kg = createSmallDataset();
// make first point wrong:
auto pos = kg.find("POINT(");
kg = kg.insert(pos + 7, "wrongStuff");
Expand Down Expand Up @@ -339,11 +340,13 @@ class SpatialJoinParamTest
: "\"Statue of liberty\"";
std::string name5 =
(std::get<4>(GetParam())) ? "\"eiffel tower Area\"" : "\"eiffel tower\"";

std::string node1 = (std::get<4>(GetParam())) ? "<nodeArea_1>" : "<node_1>";
std::string node2 = (std::get<4>(GetParam())) ? "<nodeArea_2>" : "<node_2>";
std::string node3 = (std::get<4>(GetParam())) ? "<nodeArea_3>" : "<node_3>";
std::string node4 = (std::get<4>(GetParam())) ? "<nodeArea_4>" : "<node_4>";
std::string node5 = (std::get<4>(GetParam())) ? "<nodeArea_5>" : "<node_5>";

std::string geometry1 =
(std::get<4>(GetParam())) ? "<geometryArea1>" : "<geometry1>";
std::string geometry2 =
Expand All @@ -354,6 +357,7 @@ class SpatialJoinParamTest
(std::get<4>(GetParam())) ? "<geometryArea4>" : "<geometry4>";
std::string geometry5 =
(std::get<4>(GetParam())) ? "<geometryArea5>" : "<geometry5>";

std::string wktString1 = (std::get<4>(GetParam()))
? SpatialJoinTestHelpers::areaUniFreiburg
: "POINT(7.835050 48.012670)";
Expand Down Expand Up @@ -402,18 +406,8 @@ class SpatialJoinParamTest
Row expectedDistSelf{"0"};

// helper functions
// auto P = [](double x, double y) { return GeoPoint(y, x); }; // TODO:
// delete this line
GeoPoint P(double x, double y) { return GeoPoint(y, x); }

/* TODO: delete this lambda
auto expectedDist = [](const GeoPoint& p1, const GeoPoint& p2) {
auto p1_ = S2Point{S2LatLng::FromDegrees(p1.getLat(), p1.getLng())};
auto p2_ = S2Point{S2LatLng::FromDegrees(p2.getLat(), p2.getLng())};
return std::to_string(S2Earth::ToKm(S1Angle(p1_, p2_)));
};*/

std::string expectedDist(const GeoPoint& p1, const GeoPoint& p2) {
auto p1_ = S2Point{S2LatLng::FromDegrees(p1.getLat(), p1.getLng())};
auto p2_ = S2Point{S2LatLng::FromDegrees(p2.getLat(), p2.getLng())};
Expand Down
6 changes: 3 additions & 3 deletions test/engine/SpatialJoinTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ INSTANTIATE_TEST_SUITE_P(SpatialJoin, SpatialJoinKnownEmptyTest,
namespace resultSortedOn {

TEST(SpatialJoin, resultSortedOn) {
std::string kg = createSmallDatasetWithPoints();
std::string kg = createSmallDataset();

ad_utility::MemorySize blocksizePermutations = 16_MB;
auto qec = getQec(kg, true, true, false, blocksizePermutations, false);
Expand Down Expand Up @@ -727,7 +727,7 @@ class SpatialJoinMultiplicityAndSizeEstimateTest
};

const double doubleBound = 0.00001;
std::string kg = createSmallDatasetWithPoints();
std::string kg = createSmallDataset();

// add multiplicities to test knowledge graph
kg += "<node_1> <name> \"testing multiplicity\" .";
Expand Down Expand Up @@ -859,7 +859,7 @@ class SpatialJoinMultiplicityAndSizeEstimateTest
// ================================ here the children are only index
// scans, as they are perfectly predictable in relation to size and
// multiplicity estimates
std::string kg = createSmallDatasetWithPoints();
std::string kg = createSmallDataset();

// add multiplicities to test knowledge graph
kg += "<geometry1> <asWKT> \"POINT(7.12345 48.12345)\".";
Expand Down
20 changes: 10 additions & 10 deletions test/engine/SpatialJoinTestHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -250,17 +250,17 @@ inline std::vector<std::string> createRowVectorFromColumnVector(
return result;
}

// create a small test dataset, which focuses on points as geometry objects.
// Note, that some of these objects have a polygon representation, but for
// testing purposes, they get represented as a point here. I took those
// create a small test dataset, which focuses on points or polygons as geometry
// objects. Note, that some of these objects have a polygon representation, but
// when choosing points, they get represented a single point. I took those
// points, such that it is obvious, which pair of objects should be included,
// when the maximum distance is x meters. Please note, that these datapoints
// are not copied from a real input file. Copying the query will therefore
// likely not result in the same results as here (also the names, coordinates,
// etc. might be different in the real datasets). The updated method also
// supports polygons as areas, which can be added to the knowledge graph using
// a boolean flag
inline std::string createSmallDatasetWithPoints(bool usePolygons = false) {
// are only partially copied from a real input file. Copying the query will
// therefore likely not result in the same results as here (the names,
// coordinates, etc. might be different in the real datasets). If usePolygons is
// set to false, all objects are represented by a point, otherwise all objects
// are represented by their area.
inline std::string createSmallDataset(bool usePolygons = false) {
auto addPoint = [](std::string& kg, std::string number, std::string name,
std::string point) {
kg += absl::StrCat("<node_", number, "> <name> ", name, " .<node_", number,
Expand Down Expand Up @@ -294,7 +294,7 @@ inline std::string createSmallDatasetWithPoints(bool usePolygons = false) {
}

inline QueryExecutionContext* buildTestQEC(bool useAreas = false) {
std::string kg = createSmallDatasetWithPoints(useAreas);
std::string kg = createSmallDataset(useAreas);
ad_utility::MemorySize blocksizePermutations = 16_MB;
auto qec =
ad_utility::testing::getQec(kg, true, true, false, blocksizePermutations,
Expand Down
2 changes: 1 addition & 1 deletion test/util/IndexTestHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Index makeIndexWithTestSettings(ad_utility::MemorySize parserBufferSize) {
index.memoryLimitIndexBuilding() = 50_MB;
index.parserBufferSize() =
parserBufferSize; // Note that the default value remains unchanged, but
// some test (i.e. polygon testing in Spatial Joins)
// some tests (i.e. polygon testing in Spatial Joins)
// require a larger buffer size
return index;
}
Expand Down
4 changes: 3 additions & 1 deletion test/util/IndexTestHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ Index makeTestIndex(const std::string& indexBasename,
// Return a static `QueryExecutionContext` that refers to an index that was
// build using `makeTestIndex` (see above). The index (most notably its
// vocabulary) is the only part of the `QueryExecutionContext` that is actually
// relevant for these tests, so the other members are defaulted.
// relevant for these tests, so the other members are defaulted. Using
// the parameter parserBufferSize the buffer size can be increased, when needed
// for larger tests (like polygon testing in Spatial Joins).
QueryExecutionContext* getQec(
std::optional<std::string> turtleInput = std::nullopt,
bool loadAllPermutations = true, bool usePatterns = true,
Expand Down

0 comments on commit c1dc963

Please sign in to comment.