JuliaManifolds / ManifoldsBase.jl

Basic interface for manifolds in Julia
https://juliamanifolds.github.io/ManifoldsBase.jl/
MIT License
87 stars 8 forks source link

get_vectors might not be implemented for power manifolds #107

Closed wdiepeveen closed 2 years ago

wdiepeveen commented 2 years ago

Hi,

I'm trying to get orthonormal basis vectors on (S^2)^30, but the get_vectors function seems to give an issue. Consider the minimal example: For a non-power manifold

pixelM = Sphere(2)
pixelm = [1.0, 0.0, 0.0]
pixel_basis = get_basis(pixelM,pixelm,DefaultOrthonormalBasis())
pixel_vectors = get_vectors(pixelM,pixelm,pixel_basis)

works, but

num_pixels = 30
M = PowerManifold(pixelM, NestedPowerRepresentation(), num_pixels)
m = fill(pixelm, (num_pixels,))
basis = get_basis(M,m,DefaultOrthonormalBasis())
vectors = get_vectors(M,m,basis)

doesn't work and I get the error

ERROR: MethodError: no method matching _get_vectors(::CachedBasis{ℝ, DefaultOrthonormalBasis{ℝ, ManifoldsBase.TangentSpaceType}, ManifoldsBase.PowerBasisData{Vector{CachedBasis{ℝ, DefaultOrthonormalBasis{ℝ, ManifoldsBase.TangentSpaceType}, Vector{Vector{Float64}}}}}})
Closest candidates are:
  _get_vectors(::AbstractManifold, ::Any, ::CachedBasis) at ~/.julia/packages/ManifoldsBase/L10sL/src/bases.jl:757
  _get_vectors(::CachedBasis{𝔽, <:AbstractBasis, <:AbstractArray}) where 𝔽 at ~/.julia/packages/ManifoldsBase/L10sL/src/bases.jl:760
  _get_vectors(::CachedBasis{𝔽, <:AbstractBasis, <:DiagonalizingBasisData}) where 𝔽 at ~/.julia/packages/ManifoldsBase/L10sL/src/bases.jl:761
Stacktrace:
 [1] _get_vectors(#unused#::PowerManifold{ℝ, Sphere{2, ℝ}, Tuple{30}, NestedPowerRepresentation}, #unused#::Vector{Vector{Float64}}, B::CachedBasis{ℝ, DefaultOrthonormalBasis{ℝ, ManifoldsBase.TangentSpaceType}, ManifoldsBase.PowerBasisData{Vector{CachedBasis{ℝ, DefaultOrthonormalBasis{ℝ, ManifoldsBase.TangentSpaceType}, Vector{Vector{Float64}}}}}})
   @ ManifoldsBase ~/.julia/packages/ManifoldsBase/L10sL/src/bases.jl:758
 [2] get_vectors(M::PowerManifold{ℝ, Sphere{2, ℝ}, Tuple{30}, NestedPowerRepresentation}, p::Vector{Vector{Float64}}, B::CachedBasis{ℝ, DefaultOrthonormalBasis{ℝ, ManifoldsBase.TangentSpaceType}, ManifoldsBase.PowerBasisData{Vector{CachedBasis{ℝ, DefaultOrthonormalBasis{ℝ, ManifoldsBase.TangentSpaceType}, Vector{Vector{Float64}}}}}})
   @ ManifoldsBase ~/.julia/packages/ManifoldsBase/L10sL/src/bases.jl:755
 [3] top-level scope
   @ none:1

Am I doing something wrong here or is the get_vectors function not implemented for power manifolds? Im running ManifoldsBase v0.13.7 and Manifolds v0.8.2 on Julia 1.7.2.

mateuszbaran commented 2 years ago

Hi! get_vectors is currently not implemented for PowerManifold, mostly because I thought it's something that should be avoided (it's quite expensive and we have get_vector and get_coordinates) but it wouldn't be too hard to add.

wdiepeveen commented 2 years ago

Ah I didn't think about the cost. I'll try with get_vector first than

mateuszbaran commented 2 years ago

Anyway, I've made a pull request that implements this: #108 :slightly_smiling_face:

kellertuer commented 2 years ago

Ah yes, the cost might be large, especially memory-wise, since it would be of size n^2 (manifold dimension n). Still great to have the function in general, but for the specific topic here, we might even be better of avoiding it.