Feature/distance gradient #13
Conversation
Codecov Report
@@ Coverage Diff @@
## main #13 +/- ##
==========================================
- Coverage 47.61% 37.73% -9.89%
==========================================
Files 7 8 +1
Lines 42 53 +11
==========================================
Hits 20 20
- Misses 22 33 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Here we go!
|
src/algorithms/distancegradient.jl
Outdated
| function _landscape!(mat, alg::DistanceGradient) | ||
| @assert maximum(alg.sources) <= length(mat) | ||
| @assert minimum(alg.sources) > 0 | ||
| indices = vcat(CartesianIndices(mat)...) |
There was a problem hiding this comment.
reduce(vcat, CartesianIndices(mat)) is a more performant version - the splatting will be very slow for matrices of any relevant size.
There was a problem hiding this comment.
I've got a number of thoughts here.
- NearestNeighbours works for points in any location. I've always thought that for regular grids, it must be possible to define nearest neighbors (kdtrees) algorithmically.
- I think the most common use case for this package is for simulation analyses - in which case you might often generate multiple random landscapes from the same object? Would it make sense for objects like this, that requires a lot of preprocessing, to have a type that catches e.g. the kdtree, making new random matrices fast?
|
I am stupid. To get the results we need, the KDTree must be built on the sources of course. So in answers to above:
So that's that. I've made up for it by making a PR against this PR (#20) which is more performant and (hopefully) has a little cleaner code. |
cleaner distance code
Do not merge - this is
DistanceGradientbut there is a problem we need to solve before.