Distance matrix moved to triangular shape. #23
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…emory footprint by 50 percent.
|
hmm, I'm pretty sure that when I last benchmarked it calculating the full square was more efficient 🤔 I'll have a closer look once I'm back from vacation |
|
Hey, we should revisit this sometime :) |
Turbo87
reviewed
Aug 21, 2020
.gitignore
Outdated
| @@ -3,3 +3,4 @@ | |||
| **/*.rs.bk | |||
| Cargo.lock | |||
| .criterion | |||
| .vscode/ | |||
There was a problem hiding this comment.
this stuff belongs in your global gitignore file (see https://docs.github.com/en/github/using-git/ignoring-files#configuring-ignored-files-for-all-repositories-on-your-computer) 😉
There was a problem hiding this comment.
Thanks for pointing that out :)
src/parallel.rs
Outdated
| @@ -4,14 +4,12 @@ cfg_if! { | |||
| if #[cfg(feature = "rayon")] { | |||
| use rayon::slice; | |||
| pub use rayon::prelude::*; | |||
|
|
|||
There was a problem hiding this comment.
any reason for removing the whitespace here?
There was a problem hiding this comment.
I guess this happened as I was trying to add another optional rayon feature. I added it again.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Distances in the distance matrix are only computed to preceding points. This speeds up distance calculation by ~50% and reduces memory footprint. For flights for which only one iteration is needed, benching indicates a speed up of around 15% overall. For flights with multiple iterations, I was not able to bench any kind of speed up. For longer flights with many fixes, the reduction of memory should be an improvement and could allow scoring of longer flights on smaller devices.
Moving the distance matrix to a triangular shape effectively only allows scoring in one direction. In the original implementation, the first iteration was scored 'backwards' to find potential starting points. The following iterations were then scored 'forward', starting at the selected points. This is not possible with the triangular matrix anymore.
Now, the first iteration is scored 'backwards' as well. Potential starting points and their corresponding solutions are identified. But we can not start at these points anymore. So after a start point is picked, valid end points for this start points are identified and when iterating backwards, only they are allowed by penalising other end points. The solution of this iteration is guaranteed to be as good as the solution when starting from start point in a forward manner. In the test flight, the exact number of iterations were needed. I believe that there are cases where the solution can even be better (as it is not limited to end at start point) which would result in less iteration.
In olc_forward, the same algorithms is provided with the other half of the triangular matrix, scoring everything the other way. For the average flight, this implementation seems to need more iterations, as the typical solution seems to be biased towards the start.