From abd82fdc9dec7bc2e861260fdb018bc3c686c194 Mon Sep 17 00:00:00 2001 From: Charles Kawczynski Date: Tue, 12 Sep 2023 09:20:11 -0700 Subject: [PATCH 1/4] Add broken test for 1453 --- test/RecursiveApply/recursive_apply.jl | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/RecursiveApply/recursive_apply.jl b/test/RecursiveApply/recursive_apply.jl index e77c824c73..5e5e2e3b91 100644 --- a/test/RecursiveApply/recursive_apply.jl +++ b/test/RecursiveApply/recursive_apply.jl @@ -2,6 +2,7 @@ using JET using Test using ClimaCore.RecursiveApply +using ClimaCore.Geometry @static if @isdefined(var"@test_opt") # v1.7 and higher @testset "RecursiveApply optimization test" begin @@ -84,3 +85,13 @@ end @inferred RecursiveApply.rmaptype((x, y) -> zero(x), nt, nt) end end + +@testset "NamedTuples and vectors" begin + FT = Float64 + nt = (; a = FT(1), b = FT(2)) + uv = Geometry.UVVector(FT(1), FT(2)) + @test_broken rz = RecursiveApply.rmap(*, nt, uv) + @test_broken typeof(rz) == + NamedTuple{(:a, :b), Tuple{UVVector{FT}, UVVector{FT}}} + @test_broken @inferred RecursiveApply.rmap(*, nt, uv) +end From d9de8843afc06aadff28f37e01cb74b9431abb72 Mon Sep 17 00:00:00 2001 From: Charles Kawczynski Date: Tue, 12 Sep 2023 09:21:34 -0700 Subject: [PATCH 2/4] Fix RecursiveApply on NamedTuples with AxisTensors --- src/RecursiveApply/RecursiveApply.jl | 15 +++++++++++++-- test/RecursiveApply/recursive_apply.jl | 13 ++++++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/RecursiveApply/RecursiveApply.jl b/src/RecursiveApply/RecursiveApply.jl index 4f277219a3..6479f74bfd 100755 --- a/src/RecursiveApply/RecursiveApply.jl +++ b/src/RecursiveApply/RecursiveApply.jl @@ -51,12 +51,23 @@ rmap(fn::F, X::NamedTuple{names}) where {F, names} = rmap(fn::F, X, Y) where {F} = fn(X, Y) rmap(fn::F, X::Tuple{}, Y::Tuple{}) where {F} = () -rmap(fn::F, X::Tuple{}, Y::Tuple) where {F} = () -rmap(fn::F, X::Tuple, Y::Tuple{}) where {F} = () +rmap(fn::F, X::Tuple{}, Y) where {F} = () +rmap(fn::F, X, Y::Tuple{}) where {F} = () rmap(fn::F, X::Tuple, Y::Tuple) where {F} = (rmap(fn, first(X), first(Y)), rmap(fn, Base.tail(X), Base.tail(Y))...) + +rmap(fn::F, X, Y::Tuple) where {F} = + (rmap(fn, X, first(Y)), rmap(fn, X, Base.tail(Y))...) + +rmap(fn::F, X::Tuple, Y) where {F} = + (rmap(fn, first(X), Y), rmap(fn, Base.tail(X), Y)...) + rmap(fn::F, X::NamedTuple{names}, Y::NamedTuple{names}) where {F, names} = NamedTuple{names}(rmap(fn, Tuple(X), Tuple(Y))) +rmap(fn::F, X::NamedTuple{names}, Y) where {F, names} = + NamedTuple{names}(rmap(fn, Tuple(X), Y)) +rmap(fn::F, X, Y::NamedTuple{names}) where {F, names} = + NamedTuple{names}(rmap(fn, X, Tuple(Y))) rmin(X, Y) = rmap(min, X, Y) diff --git a/test/RecursiveApply/recursive_apply.jl b/test/RecursiveApply/recursive_apply.jl index 5e5e2e3b91..643c2a972c 100644 --- a/test/RecursiveApply/recursive_apply.jl +++ b/test/RecursiveApply/recursive_apply.jl @@ -86,12 +86,15 @@ end end end -@testset "NamedTuples and vectors" begin +@testset "NamedTuples and axis tensors" begin FT = Float64 nt = (; a = FT(1), b = FT(2)) uv = Geometry.UVVector(FT(1), FT(2)) - @test_broken rz = RecursiveApply.rmap(*, nt, uv) - @test_broken typeof(rz) == - NamedTuple{(:a, :b), Tuple{UVVector{FT}, UVVector{FT}}} - @test_broken @inferred RecursiveApply.rmap(*, nt, uv) + @test rz = RecursiveApply.rmap(*, nt, uv) + @test typeof(rz) == NamedTuple{(:a, :b), Tuple{UVVector{FT}, UVVector{FT}}} + @test @inferred RecursiveApply.rmap(*, nt, uv) + @test rz.a.u == 1 + @test rz.a.v == 2 + @test rz.b.u == 1 + @test rz.b.v == 4 end From 4d276413fc37eaff326b37d0f750826f3ad97286 Mon Sep 17 00:00:00 2001 From: Charles Kawczynski Date: Sat, 16 Sep 2023 11:01:23 -0700 Subject: [PATCH 3/4] Fix unit test --- test/RecursiveApply/recursive_apply.jl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/RecursiveApply/recursive_apply.jl b/test/RecursiveApply/recursive_apply.jl index 643c2a972c..1ccc7cf49f 100644 --- a/test/RecursiveApply/recursive_apply.jl +++ b/test/RecursiveApply/recursive_apply.jl @@ -90,11 +90,11 @@ end FT = Float64 nt = (; a = FT(1), b = FT(2)) uv = Geometry.UVVector(FT(1), FT(2)) - @test rz = RecursiveApply.rmap(*, nt, uv) + rz = RecursiveApply.rmap(*, nt, uv) @test typeof(rz) == NamedTuple{(:a, :b), Tuple{UVVector{FT}, UVVector{FT}}} - @test @inferred RecursiveApply.rmap(*, nt, uv) + @inferred RecursiveApply.rmap(*, nt, uv) @test rz.a.u == 1 @test rz.a.v == 2 - @test rz.b.u == 1 + @test rz.b.u == 2 @test rz.b.v == 4 end From 54abed4192e80dca4e4ecb6e025e15b035b849cf Mon Sep 17 00:00:00 2001 From: Charles Kawczynski Date: Sun, 17 Sep 2023 09:23:39 -0700 Subject: [PATCH 4/4] Fix ambiguities --- src/RecursiveApply/RecursiveApply.jl | 33 ++++++++++++++++++++-------- test/aqua.jl | 2 +- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/RecursiveApply/RecursiveApply.jl b/src/RecursiveApply/RecursiveApply.jl index 6479f74bfd..b87340799f 100755 --- a/src/RecursiveApply/RecursiveApply.jl +++ b/src/RecursiveApply/RecursiveApply.jl @@ -46,8 +46,8 @@ rmap(fn::F, X) where {F} = fn(X) rmap(fn::F, X::Tuple{}) where {F} = () rmap(fn::F, X::Tuple) where {F} = (rmap(fn, first(X)), rmap(fn, Base.tail(X))...) -rmap(fn::F, X::NamedTuple{names}) where {F, names} = - NamedTuple{names}(rmap(fn, Tuple(X))) +rmap(fn::F, X::NamedTuple) where {F} = + NamedTuple{nt_names(X)}(rmap(fn, Tuple(X))) rmap(fn::F, X, Y) where {F} = fn(X, Y) rmap(fn::F, X::Tuple{}, Y::Tuple{}) where {F} = () @@ -55,20 +55,35 @@ rmap(fn::F, X::Tuple{}, Y) where {F} = () rmap(fn::F, X, Y::Tuple{}) where {F} = () rmap(fn::F, X::Tuple, Y::Tuple) where {F} = (rmap(fn, first(X), first(Y)), rmap(fn, Base.tail(X), Base.tail(Y))...) +rmap(fn::F, X::Tuple, Y::Tuple{}) where {F} = + (rmap(fn, first(X)), rmap(fn, Base.tail(X))...) +rmap(fn::F, X::Tuple{}, Y::Tuple) where {F} = + (rmap(fn, first(Y)), rmap(fn, Base.tail(Y))...) rmap(fn::F, X, Y::Tuple) where {F} = (rmap(fn, X, first(Y)), rmap(fn, X, Base.tail(Y))...) rmap(fn::F, X::Tuple, Y) where {F} = (rmap(fn, first(X), Y), rmap(fn, Base.tail(X), Y)...) -rmap(fn::F, X::NamedTuple{names}, Y::NamedTuple{names}) where {F, names} = - NamedTuple{names}(rmap(fn, Tuple(X), Tuple(Y))) -rmap(fn::F, X::NamedTuple{names}, Y) where {F, names} = - NamedTuple{names}(rmap(fn, Tuple(X), Y)) -rmap(fn::F, X, Y::NamedTuple{names}) where {F, names} = - NamedTuple{names}(rmap(fn, X, Tuple(Y))) - +function rmap(fn::F, X::NamedTuple, Y::NamedTuple) where {F} + @assert nt_names(X) === nt_names(Y) + return NamedTuple{nt_names(X)}(rmap(fn, Tuple(X), Tuple(Y))) +end +rmap(fn::F, X::NamedTuple, Y) where {F} = + NamedTuple{nt_names(X)}(rmap(fn, Tuple(X), Y)) +rmap(fn::F, X::NamedTuple, Y::Tuple) where {F} = + NamedTuple{nt_names(X)}(rmap(fn, Tuple(X), Y)) +rmap(fn::F, X::NamedTuple, Y::Tuple{}) where {F} = + NamedTuple{nt_names(X)}(rmap(fn, Tuple(X))) +rmap(fn::F, X, Y::NamedTuple) where {F} = + NamedTuple{nt_names(Y)}(rmap(fn, X, Tuple(Y))) +rmap(fn::F, X::Tuple, Y::NamedTuple) where {F} = + NamedTuple{nt_names(Y)}(rmap(fn, X, Tuple(Y))) +rmap(fn::F, X::Tuple{}, Y::NamedTuple) where {F} = + NamedTuple{nt_names(Y)}(rmap(fn, Tuple(Y))) + +nt_names(::NamedTuple{names}) where {names} = names rmin(X, Y) = rmap(min, X, Y) rmax(X, Y) = rmap(max, X, Y) diff --git a/test/aqua.jl b/test/aqua.jl index e97d004427..a62f64c068 100644 --- a/test/aqua.jl +++ b/test/aqua.jl @@ -22,7 +22,7 @@ using Aqua for method_ambiguity in ambs @show method_ambiguity end - @test length(ambs) ≤ 16 + @test length(ambs) ≤ 15 end @testset "Aqua tests (additional)" begin