From 25e95a9c72e0789300ca79986b59117e15393cd6 Mon Sep 17 00:00:00 2001 From: Ronny Bergmann Date: Wed, 15 May 2024 14:40:56 +0200 Subject: [PATCH 01/10] Fix a doc string for power --- src/PowerManifold.jl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/PowerManifold.jl b/src/PowerManifold.jl index 78daf96b..2aebdcb2 100644 --- a/src/PowerManifold.jl +++ b/src/PowerManifold.jl @@ -72,6 +72,9 @@ can be used to nest manifold, i.e. `PowerManifold(M, NestedPowerRepresentation() represents vectors of length 2 whose elements are vectors of length 3 of points on N in a nested array representation. +The third signature `M^(...)` is equivalent to the first one, and hence either yields +a combination of power manifolds to _one_ larger power manifold, or an + Since there is no default [`AbstractPowerRepresentation`](@ref) within this interface, the `^` operator is only available for `PowerManifold`s and concatenates dimensions. From 8d162437bf2d7e230188f8d767a4d4c704918072 Mon Sep 17 00:00:00 2001 From: Ronny Bergmann Date: Wed, 15 May 2024 14:41:20 +0200 Subject: [PATCH 02/10] Implement and test fill(M,p) --- NEWS.md | 6 ++++++ src/ManifoldsBase.jl | 4 ++++ src/PowerManifold.jl | 25 +++++++++++++++++++++++++ test/power.jl | 15 ++++++++++++++- 4 files changed, 49 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index cf148c70..0535e381 100644 --- a/NEWS.md +++ b/NEWS.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.15.10] 15/05/2024 + +### Added + +* functions `fill(N, p)` and `fill!(N, P, p)` to fill values into a point on a power manifold `N`. + ## [0.15.9] 02/05/2024 ### Added diff --git a/src/ManifoldsBase.jl b/src/ManifoldsBase.jl index 09054298..3c946558 100644 --- a/src/ManifoldsBase.jl +++ b/src/ManifoldsBase.jl @@ -9,6 +9,8 @@ import Base: copyto!, angle, eltype, + fill, + fill!, isempty, length, similar, @@ -1241,6 +1243,8 @@ export ×, embed!, embed_project, embed_project!, + fill, + fill!, geodesic, geodesic!, get_basis, diff --git a/src/PowerManifold.jl b/src/PowerManifold.jl index 2aebdcb2..bf5001de 100644 --- a/src/PowerManifold.jl +++ b/src/PowerManifold.jl @@ -528,6 +528,31 @@ function exp!(M::PowerManifoldNestedReplacing, q, p, X) return q end +@doc raw""" + fill(M::AbstractPowerManifold, p) + +Create a point on the [`AbstractPowerManifold`](@ref) `M`, where every entry is set to the +point `p`. +""" +function fill(M::AbstractPowerManifold, p) + P = allocate_result(M, rand) # rand finds the right way to allocate our point usually + return fill!(M, P, p) +end + +@doc raw""" + fill!(M::AbstractPowerManifold, P, p) + +Fill a point `P` on the [`AbstractPowerManifold`](@ref) `M`, where every entry is set to the +point `p`. +""" +function fill!(M::AbstractPowerManifold, P, p) + for i in get_iterator(M) + P[M, i] = p # can we do something that this is closer to fill/fill! and set the ref to p? + end + return P +end + + function get_basis(M::AbstractPowerManifold, p, B::AbstractBasis) rep_size = representation_size(M.manifold) vs = [get_basis(M.manifold, _read(M, rep_size, p, i), B) for i in get_iterator(M)] diff --git a/test/power.jl b/test/power.jl index aed5c441..ad5edfe3 100644 --- a/test/power.jl +++ b/test/power.jl @@ -334,7 +334,7 @@ struct TestArrayRepresentation <: AbstractPowerRepresentation end @test norm(N, P, Z .- Zc) ≈ 0 end - @testset "Other stuff" begin + @testset "Curvature" begin M1 = TestSphere(2) @testset "Weingarten" begin Mpr = PowerManifold(M1, NestedPowerRepresentation(), 2) @@ -388,4 +388,17 @@ struct TestArrayRepresentation <: AbstractPowerRepresentation end p = rand(N) @test zero_vector(N, p) == 0 .* p end + + @testset "fill" begin + M = ManifoldsBase.DefaultManifold(3) + N = PowerManifold(M, NestedPowerRepresentation(), 2) + p = [1.0, 2.0, 3.0] + P1 = fill(N, p) + @test P1[N, 1] == p + @test P1[N, 1] == p + P2 = [zeros(3), zeros(3)] + fill!(N, P2, p) + @test P2[N, 1] == p + @test P2[N, 1] == p + end end From 38539ad98adb462a0a438f26d35c2b093474374e Mon Sep 17 00:00:00 2001 From: Ronny Bergmann Date: Wed, 15 May 2024 14:44:17 +0200 Subject: [PATCH 03/10] adapt tests. --- test/test_aqua.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_aqua.jl b/test/test_aqua.jl index 4e1bec8f..18edc899 100644 --- a/test/test_aqua.jl +++ b/test/test_aqua.jl @@ -5,6 +5,7 @@ using Aqua, ManifoldsBase, Test ManifoldsBase; ambiguities = (exclude = [ allocate, # has many possible call patterns that are not supported and ambiguous + fill, # points will probably not be Integer or a range, so this should be fine. getindex, # ambiguous call patterns are not supported setindex!, # ambiguous call patterns are not supported ], broken = false), From 5749ce9b64039a3b25ed03b2e5edc1bf656707d6 Mon Sep 17 00:00:00 2001 From: Ronny Bergmann Date: Wed, 15 May 2024 14:55:11 +0200 Subject: [PATCH 04/10] bump version. --- Project.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Project.toml b/Project.toml index 1c1fe3ca..a2f5afa3 100644 --- a/Project.toml +++ b/Project.toml @@ -1,7 +1,7 @@ name = "ManifoldsBase" uuid = "3362f125-f0bb-47a3-aa74-596ffd7ef2fb" authors = ["Seth Axen ", "Mateusz Baran ", "Ronny Bergmann ", "Antoine Levitt "] -version = "0.15.9" +version = "0.15.10" [deps] LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" From 8c3a82b68bdd71a911e54ccb0318284c2cec8822 Mon Sep 17 00:00:00 2001 From: Ronny Bergmann Date: Thu, 16 May 2024 10:59:02 +0200 Subject: [PATCH 05/10] Remove the ambiguity. --- src/PowerManifold.jl | 21 ++++++++++++++------- test/power.jl | 8 ++++---- test/test_aqua.jl | 1 - 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/PowerManifold.jl b/src/PowerManifold.jl index bf5001de..3c72c8b1 100644 --- a/src/PowerManifold.jl +++ b/src/PowerManifold.jl @@ -529,23 +529,30 @@ function exp!(M::PowerManifoldNestedReplacing, q, p, X) end @doc raw""" - fill(M::AbstractPowerManifold, p) + fill(p, M::AbstractPowerManifold) Create a point on the [`AbstractPowerManifold`](@ref) `M`, where every entry is set to the point `p`. + +!!! note + while usually the manifold is a first argument in all functions in `ManifoldsBase.jl`, + we follow the signature of `fill`, where the power manifold serves are the size information. """ -function fill(M::AbstractPowerManifold, p) +function fill(p, M::AbstractPowerManifold) P = allocate_result(M, rand) # rand finds the right way to allocate our point usually - return fill!(M, P, p) + return fill!(P, p, M) end @doc raw""" - fill!(M::AbstractPowerManifold, P, p) + fill!(P, p, M::AbstractPowerManifold) -Fill a point `P` on the [`AbstractPowerManifold`](@ref) `M`, where every entry is set to the -point `p`. +Fill a point `P` on the [`AbstractPowerManifold`](@ref) `M`, setting every entry to `p`. + +!!! note + while usually the manifold is a first argument in all functions in `ManifoldsBase.jl`, + we follow the signature of `fill!`, where the power manifold serves are the size information. """ -function fill!(M::AbstractPowerManifold, P, p) +function fill!(P, p, M::AbstractPowerManifold) for i in get_iterator(M) P[M, i] = p # can we do something that this is closer to fill/fill! and set the ref to p? end diff --git a/test/power.jl b/test/power.jl index ad5edfe3..9e6db585 100644 --- a/test/power.jl +++ b/test/power.jl @@ -393,12 +393,12 @@ struct TestArrayRepresentation <: AbstractPowerRepresentation end M = ManifoldsBase.DefaultManifold(3) N = PowerManifold(M, NestedPowerRepresentation(), 2) p = [1.0, 2.0, 3.0] - P1 = fill(N, p) - @test P1[N, 1] == p + P1 = fill(p, N) @test P1[N, 1] == p + @test P1[N, 2] == p P2 = [zeros(3), zeros(3)] - fill!(N, P2, p) - @test P2[N, 1] == p + fill!(P2, p, N) @test P2[N, 1] == p + @test P2[N, 2] == p end end diff --git a/test/test_aqua.jl b/test/test_aqua.jl index 18edc899..4e1bec8f 100644 --- a/test/test_aqua.jl +++ b/test/test_aqua.jl @@ -5,7 +5,6 @@ using Aqua, ManifoldsBase, Test ManifoldsBase; ambiguities = (exclude = [ allocate, # has many possible call patterns that are not supported and ambiguous - fill, # points will probably not be Integer or a range, so this should be fine. getindex, # ambiguous call patterns are not supported setindex!, # ambiguous call patterns are not supported ], broken = false), From e8a622a715cece7278ae7e917264844f2d1dcd8e Mon Sep 17 00:00:00 2001 From: Mateusz Baran Date: Sun, 19 May 2024 10:05:00 +0200 Subject: [PATCH 06/10] `fill!` for array power manifold --- src/PowerManifold.jl | 11 +++++++++-- test/power.jl | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/PowerManifold.jl b/src/PowerManifold.jl index 3c72c8b1..071fdd13 100644 --- a/src/PowerManifold.jl +++ b/src/PowerManifold.jl @@ -549,12 +549,19 @@ end Fill a point `P` on the [`AbstractPowerManifold`](@ref) `M`, setting every entry to `p`. !!! note - while usually the manifold is a first argument in all functions in `ManifoldsBase.jl`, + while usually the manifold is the first argument in all functions in `ManifoldsBase.jl`, we follow the signature of `fill!`, where the power manifold serves are the size information. """ +function fill!(P, p, M::PowerManifoldNestedReplacing) + for i in get_iterator(M) + P[i...] = p # can we do something that this is closer to fill/fill! and set the ref to p? + end + return P +end function fill!(P, p, M::AbstractPowerManifold) + rep_size = representation_size(M.manifold) for i in get_iterator(M) - P[M, i] = p # can we do something that this is closer to fill/fill! and set the ref to p? + copyto!(M.manifold, _write(M, rep_size, P, i), p) end return P end diff --git a/test/power.jl b/test/power.jl index 9e6db585..90b48f7d 100644 --- a/test/power.jl +++ b/test/power.jl @@ -1,7 +1,12 @@ using Test using ManifoldsBase using ManifoldsBase: - AbstractNumbers, ℝ, ℂ, NestedReplacingPowerRepresentation, VectorSpaceType + AbstractNumbers, + DefaultManifold, + ℝ, + ℂ, + NestedReplacingPowerRepresentation, + VectorSpaceType using StaticArrays using LinearAlgebra using Random @@ -24,6 +29,23 @@ end struct TestArrayRepresentation <: AbstractPowerRepresentation end +const TestPowerManifoldMultidimensional = + AbstractPowerManifold{𝔽,<:AbstractManifold{𝔽},TestArrayRepresentation} where {𝔽} + +function ManifoldsBase.representation_size(M::TestPowerManifoldMultidimensional) + return (representation_size(M.manifold)..., ManifoldsBase.get_parameter(M.size)...) +end + +@inline function ManifoldsBase._write( + ::TestPowerManifoldMultidimensional, + rep_size::Tuple, + x::AbstractArray, + i::Tuple, +) + return view(x, ManifoldsBase.rep_size_to_colons(rep_size)..., i...) +end + + @testset "Power Manifold" begin @testset "Power Manifold with a test representation" begin @@ -400,5 +422,14 @@ struct TestArrayRepresentation <: AbstractPowerRepresentation end fill!(P2, p, N) @test P2[N, 1] == p @test P2[N, 2] == p + + NAR = PowerManifold(M, TestArrayRepresentation(), 2) + P1 = fill(p, NAR) + @test P1 isa Matrix{Float64} + @test P1 == [1.0 1.0; 2.0 2.0; 3.0 3.0] + + P2 = similar(P1) + fill!(P2, p, NAR) + @test P2 == [1.0 1.0; 2.0 2.0; 3.0 3.0] end end From 5b89f68f56182749f31d51643f0fd2d5f4b19996 Mon Sep 17 00:00:00 2001 From: Ronny Bergmann Date: Sun, 19 May 2024 10:11:54 +0200 Subject: [PATCH 07/10] Remove a comment. --- src/PowerManifold.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PowerManifold.jl b/src/PowerManifold.jl index 3c72c8b1..7f44de13 100644 --- a/src/PowerManifold.jl +++ b/src/PowerManifold.jl @@ -554,7 +554,7 @@ Fill a point `P` on the [`AbstractPowerManifold`](@ref) `M`, setting every entry """ function fill!(P, p, M::AbstractPowerManifold) for i in get_iterator(M) - P[M, i] = p # can we do something that this is closer to fill/fill! and set the ref to p? + P[M, i] = p end return P end From a40affaf6496f2994875fcf110d31fc884c292df Mon Sep 17 00:00:00 2001 From: Ronny Bergmann Date: Sun, 19 May 2024 10:13:49 +0200 Subject: [PATCH 08/10] finish a sentence. --- src/PowerManifold.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PowerManifold.jl b/src/PowerManifold.jl index c894bb2a..aded57a0 100644 --- a/src/PowerManifold.jl +++ b/src/PowerManifold.jl @@ -73,7 +73,7 @@ represents vectors of length 2 whose elements are vectors of length 3 of points in a nested array representation. The third signature `M^(...)` is equivalent to the first one, and hence either yields -a combination of power manifolds to _one_ larger power manifold, or an +a combination of power manifolds to _one_ larger power manifold, or an power manifold with the default representation. Since there is no default [`AbstractPowerRepresentation`](@ref) within this interface, the `^` operator is only available for `PowerManifold`s and concatenates dimensions. From bfbd27b5fcd0844e03f64a3303255bba3ecad115 Mon Sep 17 00:00:00 2001 From: Mateusz Baran Date: Sun, 19 May 2024 11:20:22 +0200 Subject: [PATCH 09/10] add a test for `NestedReplacing` --- test/power.jl | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/power.jl b/test/power.jl index 90b48f7d..513b113f 100644 --- a/test/power.jl +++ b/test/power.jl @@ -423,6 +423,16 @@ end @test P2[N, 1] == p @test P2[N, 2] == p + M = ManifoldsBase.DefaultManifold(3) + NR = PowerManifold(M, NestedReplacingPowerRepresentation(), 2) + P1 = fill(p, NR) + @test P1[NR, 1] === p + @test P1[NR, 2] === p + P2 = [zeros(3), zeros(3)] + fill!(P2, p, NR) + @test P2[NR, 1] === p + @test P2[NR, 2] === p + NAR = PowerManifold(M, TestArrayRepresentation(), 2) P1 = fill(p, NAR) @test P1 isa Matrix{Float64} From 0ca3cbd361c449af56aaa6de457752100f48c167 Mon Sep 17 00:00:00 2001 From: Ronny Bergmann Date: Sun, 19 May 2024 11:58:46 +0200 Subject: [PATCH 10/10] Apply suggestions from code review Co-authored-by: Mateusz Baran --- NEWS.md | 2 +- src/PowerManifold.jl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 0535e381..1a614167 100644 --- a/NEWS.md +++ b/NEWS.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -* functions `fill(N, p)` and `fill!(N, P, p)` to fill values into a point on a power manifold `N`. +* Functions `fill(p, N)` and `fill!(P, p, N)` to fill values into a point on a power manifold `N`. ## [0.15.9] 02/05/2024 diff --git a/src/PowerManifold.jl b/src/PowerManifold.jl index aded57a0..e75266a6 100644 --- a/src/PowerManifold.jl +++ b/src/PowerManifold.jl @@ -73,7 +73,7 @@ represents vectors of length 2 whose elements are vectors of length 3 of points in a nested array representation. The third signature `M^(...)` is equivalent to the first one, and hence either yields -a combination of power manifolds to _one_ larger power manifold, or an power manifold with the default representation. +a combination of power manifolds to _one_ larger power manifold, or a power manifold with the default representation. Since there is no default [`AbstractPowerRepresentation`](@ref) within this interface, the `^` operator is only available for `PowerManifold`s and concatenates dimensions.