Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add helpful message when ProductManifold is used without RAT.jl #201

Merged
merged 4 commits into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,16 @@ 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.14] 27/08/2024

### Added

* A helpful error message when `ProductManifold` is used without `RecursiveArrayTools.jl`.

### Changed

* `representation_size` for `ProductManifold` now returns `nothing` instead of a one-element tuple. This change makes it easier to notice errors caused by not having `RecursiveArrayTools.jl` loaded.

## [0.15.13] 10/08/2024

### Changed
Expand Down
2 changes: 1 addition & 1 deletion Project.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name = "ManifoldsBase"
uuid = "3362f125-f0bb-47a3-aa74-596ffd7ef2fb"
authors = ["Seth Axen <[email protected]>", "Mateusz Baran <[email protected]>", "Ronny Bergmann <[email protected]>", "Antoine Levitt <[email protected]>"]
version = "0.15.13"
version = "0.15.14"

[deps]
LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
Expand Down
11 changes: 10 additions & 1 deletion src/ManifoldsBase.jl
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,16 @@ isomorphisms.
end
@inline function allocate_result(M::AbstractManifold, f)
T = allocate_result_type(M, f, ())
return Array{T}(undef, representation_size(M)...)
rs = representation_size(M)
if isnothing(rs)
msg = "Could not allocate result of function $f on manifold $M."
if M isa ProductManifold
msg *= " This error could be resolved by importing RecursiveArrayTools.jl. If this is not the case, please open report an issue."
end
error(msg)
else
return Array{T}(undef, rs...)
end
end

"""
Expand Down
18 changes: 14 additions & 4 deletions src/PowerManifold.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1319,7 +1319,7 @@ end

Base.@propagate_inbounds @inline function _read(
M::AbstractPowerManifold,
rep_size::Tuple,
rep_size::Union{Tuple,Nothing},
x::AbstractArray,
i::Int,
)
Expand All @@ -1328,7 +1328,7 @@ end

Base.@propagate_inbounds @inline function _read(
::Union{PowerManifoldNested,PowerManifoldNestedReplacing},
rep_size::Tuple,
rep_size::Union{Tuple,Nothing},
x::AbstractArray,
i::Tuple,
)
Expand Down Expand Up @@ -1765,15 +1765,25 @@ function Weingarten!(M::AbstractPowerManifold, Y, p, X, V)
return Y
end

@inline function _write(M::AbstractPowerManifold, rep_size::Tuple, x::AbstractArray, i::Int)
@inline function _write(
M::AbstractPowerManifold,
rep_size::Union{Tuple,Nothing},
x::AbstractArray,
i::Int,
)
return _write(M, rep_size, x, (i,))
end

@inline function _is_nested_write_getindex(::PowerManifoldNested, x)
return !isbitstype(eltype(x))
end

@inline function _write(M::PowerManifoldNested, ::Tuple, x::AbstractArray, i::Tuple)
@inline function _write(
M::PowerManifoldNested,
::Union{Tuple,Nothing},
x::AbstractArray,
i::Tuple,
)
if _is_nested_write_getindex(M, x)
return x[i...]
else
Expand Down
4 changes: 2 additions & 2 deletions src/ProductManifold.jl
Original file line number Diff line number Diff line change
Expand Up @@ -876,8 +876,8 @@ function retract!(
return q
end

function representation_size(M::ProductManifold)
return (mapreduce(m -> prod(representation_size(m)), +, M.manifolds),)
function representation_size(::ProductManifold)
return nothing
end

@doc raw"""
Expand Down
15 changes: 12 additions & 3 deletions test/product_manifold.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,21 @@ using ManifoldsBase:
AbstractNumbers, ℝ, ℂ, NestedReplacingPowerRepresentation, ProductBasisData
using LinearAlgebra
using Random
using RecursiveArrayTools

s = @__DIR__
s = (@__DIR__) * "/test/"
!(s in LOAD_PATH) && (push!(LOAD_PATH, s))
using ManifoldsBaseTestUtils

@testset "Product manifold without RecursiveArrayTools.jl" begin
M1 = TestSphere(2)
M2 = ManifoldsBase.DefaultManifold(2, 2)

M = ProductManifold(M1, M2)
@test_throws ErrorException rand(M)
end

using RecursiveArrayTools

@testset "Product manifold" begin
M1 = TestSphere(2)
M2 = ManifoldsBase.DefaultManifold(2, 2)
Expand Down Expand Up @@ -157,7 +166,7 @@ using ManifoldsBaseTestUtils

@testset "Basic operations" begin
@test manifold_dimension(M) == 6
@test representation_size(M) == (7,)
@test representation_size(M) === nothing
@test distance(M, p1, p2) ≈ 4.551637188998299
qr = similar(p1)
exp!(M, qr, p1, X1)
Expand Down
2 changes: 1 addition & 1 deletion test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ using ManifoldsBase
include("validation_manifold.jl")
include("embedded_manifold.jl")
include("test_sphere.jl")
include("product_manifold.jl")
include("power.jl")
include("domain_errors.jl")
include("vector_transport.jl")
include("metric.jl")
include("product_manifold.jl")
include("fibers.jl")
include("numerical_checks.jl")
end
Loading