Arbitrary radii for spheres

benneti commented 5 years ago

As discussed at DifferentialEquations.jl #389 it might be useful to change the Sphere class to include a radius

struct Sphere{T} <: Manifold where {T <: Real}
    r::T
end
Sphere() = Sphere(1)
retract!(S::Sphere, x) = (x .= x .* (S.r / norm(x)))

I think this might be a good idea as it does not add much complexity and eases the use of the library for non normalized problems.

pkofod commented 5 years ago

I will wait to hear Antoine if he thinks it fits. Really, many of these manifolds only require 3+2 lines of code, so if we have Sphere, might might as well make it a bit general, but I won't really put my foot down.

I will say, that if you want absolute performance here, you probably don't want to do it like this. The following will allow the unit radius to be compiled completely away

struct Sphere{T} <: Manifold
    r::T
end
Sphere() = Sphere(nothing)
retract!(S::Sphere{<:Nothing}, x) = normalize!(x)
retract!(S::Sphere, x) = (x .= x .* (S.r / norm(x)))

Just a general Julia tip :) [only really matters if you do many retractions of course, but...]

benneti commented 5 years ago

Oh that is cool to know, would you say the best learning source is the official doc? Because for now I just tried stuff and read a few parts of the doc and the wiki book.

antoine-levitt commented 5 years ago

More code, more bugs, more stuff to document and change if we change an API. But sure, if you feel it's useful, can you make a PR? Don't forget project_tangent as well.

JuliaNLSolvers / ManifoldProjections.jl

Arbitrary radii for spheres #2