Skip to content

Base.length dispatch for ArrayPartition (StaticArrays) - #220

Merged
ChrisRackauckas merged 5 commits into
SciML:masterfrom
dehann:patch-1
Jul 17, 2022
Merged

Base.length dispatch for ArrayPartition (StaticArrays)#220
ChrisRackauckas merged 5 commits into
SciML:masterfrom
dehann:patch-1

Conversation

@dehann

@dehann dehann commented Jul 15, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

This only works for partition elements that themselves have a length-on-type function, e.g. SVector or SMatrix.

I'm trying to use this line of code from NearestNeighbors.jl (issue on the call length(::Type{ArrayPartition...}}):
https://github.com/KristofferC/NearestNeighbors.jl/blob/fb84d15b96a2d7f94f691db4ff6f42e994194c93/src/tree_data.jl#L14

MWE I'm working to fix:

using Manifolds, NearestNeighbors, Distances, StaticArrays
import Manifolds: ArrayPartition

Base.@kwdef struct DistSE2 <: Distances.Metric
  M = SpecialEuclidean(2)
end 
(d::DistSE2)(p,q) = Manifolds.ManifoldsBase.distance(d.M,p,q)

pts = [ArrayPartition(SA[randn(2)...], SMatrix{2,2}([1 0; 0 1.])) for _ in 1:10]
dSE2 = DistSE2()

bt = NearestNeighbors.BallTree(pts, dSE2)

# adding this PR corrects the error
# Base.length(::Type{<:ArrayPartition{F,T}}) where {F,T <: Tuple} = T.parameters .|> length |> sum
julia> bt = NearestNeighbors.BallTree(pts, dSE2)
ERROR: MethodError: no method matching length(::Type{ArrayPartition{Float64, Tuple{SVector{2, Float64}, SMatrix{2, 2, Float64, 4}}}})
Closest candidates are:
  length(::Union{Base.KeySet, Base.ValueIterator}) at /usr/local/share/julia-1.7.3/share/julia/base/abstractdict.jl:58
  length(::Union{DataStructures.OrderedRobinDict, DataStructures.RobinDict}) at ~/.julia/packages/DataStructures/59MD0/src/ordered_robin_dict.jl:86
  length(::Union{DataStructures.SortedDict, DataStructures.SortedMultiDict, DataStructures.SortedSet}) at ~/.julia/packages/DataStructures/59MD0/src/container_loops.jl:322
  ...
Stacktrace:
 [1] NearestNeighbors.TreeData(data::Vector{ArrayPartition{Float64, Tuple{SVector{2, Float64}, SMatrix{2, 2, Float64, 4}}}}, leafsize::Int64)
   @ NearestNeighbors ~/.julia/packages/NearestNeighbors/VZzTb/src/tree_data.jl:14
 [2] BallTree(data::Vector{ArrayPartition{Float64, Tuple{SVector{2, Float64}, SMatrix{2, 2, Float64, 4}}}}, metric::DistSE2; leafsize::Int64, reorder::Bool, storedata::Bool, reorderbuffer::Vector{ArrayPartition{Float64, Tuple{SVector{2, Float64}, SMatrix{2, 2, Float64, 4}}}})
   @ NearestNeighbors ~/.julia/packages/NearestNeighbors/VZzTb/src/ball_tree.jl:39
 [3] BallTree(data::Vector{ArrayPartition{Float64, Tuple{SVector{2, Float64}, SMatrix{2, 2, Float64, 4}}}}, metric::DistSE2)
   @ NearestNeighbors ~/.julia/packages/NearestNeighbors/VZzTb/src/ball_tree.jl:37
 [4] top-level scope
   @ REPL[11]:1

Currently there are further errors that I'm working separately.

xref: Using Manifolds.jl objects represented as ArrayPartition,

@dehann
Base.length dispatch for ArrayPartition
93d84ed 
This only works for partition elements that themselves have a length-on-type function, e.g. SVector or SMatrix.
@dehann

dehann commented Jul 15, 2022

Copy link
Copy Markdown
Contributor Author

cc @mateuszbaran , @Affie

@dehann dehann mentioned this pull request Jul 15, 2022
Closed
mateuszbaran
mateuszbaran reviewed Jul 15, 2022
View reviewed changes
Comment thread src/array_partition.jl Outdated
@mateuszbaran

mateuszbaran commented Jul 15, 2022

Copy link
Copy Markdown
Contributor

Yes, that seems necessary for NearestNeighbors.jl. Here is a test that can be added for that method:

p3 = ArrayPartition(SA[1.0, 2.0], MMatrix{2,2}([3.0 4.0; 3.0 5.0]))
@test length(typeof(p3)) == 6

@dehann

dehann commented Jul 15, 2022

Copy link
Copy Markdown
Contributor Author

Thanks, I added the test too!

mateuszbaran
mateuszbaran reviewed Jul 15, 2022
View reviewed changes
Comment thread test/partitions_and_static_arrays.jl Outdated
dehann and others added 2 commits July 15, 2022 13:46
@dehann

dehann commented Jul 15, 2022

Copy link
Copy Markdown
Contributor Author

I included the suggestions also thanks

@mateuszbaran

mateuszbaran commented Jul 15, 2022

Copy link
Copy Markdown
Contributor

You're welcome 🙂 .

@dehann dehann changed the title Base.length dispatch for ArrayPartition Base.length dispatch for ArrayPartition (StaticArrays) Jul 15, 2022
@dehann dehann mentioned this pull request Jul 16, 2022
Closed
ChrisRackauckas
ChrisRackauckas reviewed Jul 16, 2022
View reviewed changes
Comment thread src/array_partition.jl Outdated
@dehann @ChrisRackauckas
Update src/array_partition.jl
7e0f1a4 
Co-authored-by: Christopher Rackauckas <[email protected]>
@ChrisRackauckas
ChrisRackauckas merged commit 1dd280b into SciML:master Jul 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ChrisRackauckas ChrisRackauckas ChrisRackauckas left review comments
+1 more reviewer
@mateuszbaran mateuszbaran mateuszbaran left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

See More

Successfully merging this pull request may close these issues.

3 participants

@dehann @mateuszbaran @ChrisRackauckas