Feature/nav 32 range search in kd tree #13

clukas1 · 2024-05-12T19:52:04Z

Final pull request for KD Tree implementation into main.

NAV-23 - Simple KD Tree Implementation (already reviewed by @munterfi , @clukas1)
NAV-29 - KD Tree Builder to ensure balancing (already reviewed by @munterfi)
NAV-30 - Addition of unittests (already reviewed by @Brunner246, @clukas1)
NAV-32 - Addition of range search (new)
NAV-34 - Integration into GTFS Module (Builder and Schedule)

… Nonetheless, it’s more accurate in calculating the distance between long distances

…e/NAV-23-kd-tree-for-stops

…TITUDE`

- implement generic approach for KD-tree - simplified interface of Coordinates into `TwoDimensionalCoordinates` . This allows us to handle cartesian and geographic coordinate system - use generic name for `X` `Y` and `LONG` `LAT` - remove redundant functions in utils

…on) to KDTree.

… attributes/classes.

…/naviqore/raptor into feature/NAV-30-write-unit-tests-for-KD-Tree # Conflicts: # src/main/java/ch/naviqore/gtfs/schedule/model/Coordinate.java

- create `StopFacilityMock` for TestCase - create `KDTree` test cases for finding nearest neighbour

… of locations.

…to add locations.

…for KDTree builder.

Nav 29 balanced kd tree

…for-KD-Tree Feature/nav 30 write unit tests for kd tree

…' into feature/NAV-32-range-search-in-kd-tree # Conflicts: # src/main/java/ch/naviqore/utils/spatial/KDTree.java # src/test/java/ch/naviqore/utils/spatial/KDTreeBuilderTest.java

… positive radius values.

- Private classes and enums are not visible anymore in public methods as parameters or return types - Move KDNode and KDUtils to inner members of KDTree, since they will not be used from the outside, except for testing. - Rename MockCoordinate to CartesianCoordinate and move all coordinate types into the spatial package. - Move tree into its own index package. - Add javadoc to public interfaces, classes and methods.

munterfi · 2024-05-15T12:14:10Z

Looks good! And indeed very performant, unnoticeable when run with the benchmark! 🚀

There are some issues with out of scope visiblity of KDNode and internal Coordinates, therefore i refactored this in #14. (The PR is into this branch). Please have a look @clukas1 @Brunner246

There are some further points we should discuss on friday, concerning the style guide:

consistent toString format
which assertions to use, or should we allow mixing (jupiter, assertJ, ...)
javadoc, what should be documented? But still keep it as simple as possible.

munterfi

See comment above 😄

Brunner246

thanks for your effort =)

…-tree

…NAV-32-range-search-in-kd-tree # Conflicts: # src/main/java/ch/naviqore/gtfs/schedule/model/GtfsSchedule.java # src/main/java/ch/naviqore/gtfs/schedule/model/GtfsScheduleBuilder.java # src/main/java/ch/naviqore/gtfs/schedule/model/Stop.java

Brunner246 and others added 30 commits April 28, 2024 09:29

start working on spatial dk tree implementation

0b603d7

first implementation of DK Tree **untested** yet

422b54b

implement haversine formula - it views the Earth as a perfect sphere.…

f8fca09

… Nonetheless, it’s more accurate in calculating the distance between long distances

add haversine helper function

7308903

Merge branch 'main' of https://github.com/naviqore/raptor into featur…

9d52e5e

…e/NAV-23-kd-tree-for-stops

ENH: NAV-23- Clean code refactorings

8b8216e

ENH: NAV-23- remove test code

aa80ccb

ENH: NAV-23- proposal to keep the design open for extension

30878ac

ENH: NAV-23- rename CoordinatesType from X Y to LONGITUDE `LA…

a52af64

…TITUDE`

ENH: NAV-23

a227160

- implement generic approach for KD-tree - simplified interface of Coordinates into `TwoDimensionalCoordinates` . This allows us to handle cartesian and geographic coordinate system - use generic name for `X` `Y` and `LONG` `LAT` - remove redundant functions in utils

ENH: NAV-23 - make KDNode generic without any bounds

b97eab5

REFACTOR: NAV-23 - Add getCoordinateValue to CoordinatesType Enum.

774138b

ENH: NAV-23 - Add ability to add objects that have Coordinate (Locati…

9add21e

…on) to KDTree.

REFACTOR: NAV-23 - Moved KDTree to utils.spatial and some renaming of…

016fb35

… attributes/classes.

STYLE: NAV-23 - Remove redundant type constraint.

05cfe6c

TEST: NAV-30 - add simple unit test for KDTree

fcd9bce

Merge branch 'feature/NAV-23-kd-tree-for-stops' of https://github.com…

4a06b46

…/naviqore/raptor into feature/NAV-30-write-unit-tests-for-KD-Tree # Conflicts: # src/main/java/ch/naviqore/gtfs/schedule/model/Coordinate.java

TEST: NAV-30 - make Coordinate comparable

3a4a20d

- create `StopFacilityMock` for TestCase - create `KDTree` test cases for finding nearest neighbour

ENH: NAV-29 - Add KDTreeBuilder to builed balanced KDTrees from lists…

ba43a6a

… of locations.

TEST: NAV-23 - Add test class for KDTreeBuilder.

07c72f4

REFACTOR: NAV-29 - Remove unused methods from test class.

5231c42

REFACTOR: NAV-29 - Remove unused import.

881dd42

ENH: NAV-29 - Hide ArrayList logic from outside by adding add method …

c66d865

…to add locations.

ENH: NAV-29 - Always throw exceptions when null arguments are passed …

fe2a2d4

…for KDTree builder.

TEST: NAV-29 - Update tests to work with new KDTreeBuilder structure.

b7c26e4

REFACTOR: NAV-30 - Move tests to utils.spatial

bb9ccd1

TEST: NAV-30 - Add some new test cases.

a342fcb

FIX: NAV-30 - Fix failing tests.

49449f3

TEST: NAV-30 - Remove dependency to GTFS module in utils.spatial tests.

706ba9e

Merge pull request #12 from naviqore/NAV-29-balanced-kd-tree

eb950c3

Nav 29 balanced kd tree

clukas1 added 8 commits May 12, 2024 15:56

FIX: NAV-32 - Fix stupid error message.

53986a3

ENH: NAV-34 - Integrate KDTree into GTFS Schedule.

aef1483

TEST: NAV-30 - Make test names more consistent.

d74f9d5

Merge pull request #10 from naviqore/feature/NAV-30-write-unit-tests-…

fa35cd9

…for-KD-Tree Feature/nav 30 write unit tests for kd tree

Merge remote-tracking branch 'origin/feature/NAV-23-kd-tree-for-stops…

7d9e900

…' into feature/NAV-32-range-search-in-kd-tree # Conflicts: # src/main/java/ch/naviqore/utils/spatial/KDTree.java # src/test/java/ch/naviqore/utils/spatial/KDTreeBuilderTest.java

TEST: NAV-32 - Add tests for range search.

fdd3949

ENH: NAV-32 - Add more input validation in range search to allow only…

8bb6c46

… positive radius values.

TEST: NAV-32 - Use @nested to group tests by method.

f5329a7

clukas1 requested review from munterfi and Brunner246 and removed request for munterfi and Brunner246 May 12, 2024 19:52

clukas1 and others added 6 commits May 12, 2024 21:58

REFACTOR: NAV-23 - Remove todos in gtfs coordinate.

ab85532

TEST: NAV-32 - Add test for GTFS Coordinate implementation.

ef6cb31

ENH: NAV-32 - Add coordinate validation when constructing coordinates.

085ea97

FIX: NAV-32 - Fix harvesine distance calculation.

e1ffee9

ORG: NAV-32 - Remove default code style, use project specific

c5bac72

munterfi approved these changes May 15, 2024

View reviewed changes

STYLE: NAV-32 - Extract constant for TOLERANCE in tests

e3d953a

Brunner246 approved these changes May 15, 2024

View reviewed changes

clukas1 and others added 5 commits May 15, 2024 22:55

TEST: NAV-32 - Add more coordinate tests.

2531356

ENH: NAV-32 - Add more argument validation for coordinate classes.

442bc87

Merge pull request #14 from naviqore/review/NAV-32-range-search-in-kd…

f499cd7

…-tree

ORG: NAV-32 - Solve merge conflicts

1c8a1f8

munterfi merged commit a54369c into main May 16, 2024
1 check passed

munterfi deleted the feature/NAV-32-range-search-in-kd-tree branch May 16, 2024 11:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/nav 32 range search in kd tree #13

Feature/nav 32 range search in kd tree #13

clukas1 commented May 12, 2024

munterfi commented May 15, 2024 •

edited

Loading

munterfi left a comment

Brunner246 left a comment

Feature/nav 32 range search in kd tree #13

Feature/nav 32 range search in kd tree #13

Conversation

clukas1 commented May 12, 2024

munterfi commented May 15, 2024 • edited Loading

munterfi left a comment

Choose a reason for hiding this comment

Brunner246 left a comment

Choose a reason for hiding this comment

munterfi commented May 15, 2024 •

edited

Loading