Simplify union for spaces (#139)

* Simplify union for spaces

* Add ambiguous union tests

* ambiguity in empty/any domain unions

Author: jishnub

.github/workflows/ci.yml

@@ -1,4 +1,9 @@
 name: CI
+
+concurrency:
+  group: build-${{ github.event.pull_request.number || github.ref }}
+  cancel-in-progress: true
+
 on:
   create:
     tags:

src/Domain.jl

@@ -1,5 +1,3 @@
-
-
 export Domain, SegmentDomain, tocanonical, fromcanonical, fromcanonicalD, ∂
 export chebyshevpoints, fourierpoints, isambiguous, arclength
 export components, component, ncomponents
@@ -21,6 +19,8 @@ struct EmptyDomain <: Domain{Nothing} end
 ==(::AnyDomain, ::AnyDomain) = true
 ==(::EmptyDomain, ::EmptyDomain) = true
 
+const AnyOrEmptyDomain = Union{AnyDomain, EmptyDomain}
+
 isambiguous(::AnyDomain) = true
 dimension(::AnyDomain) = 1
 dimension(::EmptyDomain) = 0
@@ -32,21 +32,21 @@ arclength(::DomainSets.EmptySpace) = false
 
 isempty(::AnyDomain) = false
 
-reverseorientation(a::Union{AnyDomain,EmptyDomain}) = a
+reverseorientation(a::AnyOrEmptyDomain) = a
 
-canonicaldomain(a::Union{AnyDomain,EmptyDomain}) = a
+canonicaldomain(a::AnyOrEmptyDomain) = a
 
 indomain(x::Domain,::EmptyDomain) = false
 
 convert(::Type{Domain{T}}, ::AnyDomain) where T = Domain(T)
 
+# empty domain should have priority over over any domain
+_promoteanyemptydomain(::AnyDomain, ::AnyDomain) = AnyDomain()
+_promoteanyemptydomain(::AnyOrEmptyDomain, ::AnyOrEmptyDomain) = EmptyDomain()
 
-union(::AnyDomain, d::Domain) = d
-union(d::Domain, ::AnyDomain) = d
-
-union(::EmptyDomain, ::EmptyDomain) = EmptyDomain()
-union(::EmptyDomain, a::Domain) = a
-union(a::Domain, ::EmptyDomain) = a
+union(a::AnyOrEmptyDomain, b::AnyOrEmptyDomain) = _promoteanyemptydomain(a, b)
+union(::AnyOrEmptyDomain, a::Domain) = a
+union(a::Domain, ::AnyOrEmptyDomain) = a
 
 ##General routines
 isempty(::EmptyDomain) = true

src/Space.jl

@@ -145,22 +145,14 @@ for op in (:tocanonical,:fromcanonical,:tocanonicalD,:fromcanonicalD,:invfromcan
     @eval ($op)(sp::Space,x...)=$op(domain(sp),x...)
 end
 
-mappoint(a::Space,b::Space,x)=mappoint(domain(a),domain(b),x)
-mappoint(a::Space,b::Domain,x)=mappoint(domain(a),b,x)
-mappoint(a::Domain,b::Space,x)=mappoint(a,domain(b),x)
-
+_domain(s::Space) = domain(s)
+_domain(s) = s
+mappoint(a, b, x) = mappoint(map(_domain, (a, b, x))...)
 
+_conversion_rule(a, b) = spacescompatible(a, b) ? a : NoSpace()
 
 for FUNC in (:conversion_rule,:maxspace_rule,:union_rule)
-    @eval begin
-        function $FUNC(a,b)
-            if spacescompatible(a,b)
-                a
-            else
-                NoSpace()
-            end
-        end
-    end
+    @eval $FUNC(a, b) = _conversion_rule(a, b)
 end
 
 
@@ -250,8 +242,8 @@ end
 # this is used primarily for addition of two funs
 # that may be incompatible
 union(a::AmbiguousSpace, b::AmbiguousSpace) = b
-union(a::AmbiguousSpace, b::Space) = b
-union(a::Space, b::AmbiguousSpace) = a
+union_by_union_rule(a::AmbiguousSpace, b::Space) = b
+union_by_union_rule(a::Space, b::AmbiguousSpace) = a
 
 
 function union_by_union_rule(a::Space,b::Space)
@@ -287,12 +279,7 @@ function union(a::Space, b::Space)
     a ⊕ b
 end
 
-union(a::Space) = a
-
-union(a::Space,b::Space,c::Space) = union(union(a,b),c)
-union(a::Space,b::Space,c::Space,d::Space...) =
-    union(union(a,b),c,d...)
-
+union(a::Space, bs::Space...) = foldl(union, bs, init = a)
 
 # tests whether a Conversion operator exists
 hasconversion(a,b) = maxspace(a,b) == b

test/SpacesTest.jl

@@ -25,6 +25,11 @@ import ApproxFunBase: PointSpace, HeavisideSpace, PiecewiseSegment, dimension, V
             fany = convert(T, f)
             @test fany isa T
             @test (@inferred oftype(f, fany)) isa typeof(f)
+
+            # some trivial cases
+            s = PointSpace(1:3)
+            @test conversion_type(s, s) == s
+            @test maxspace(s, s) == s
         end
 
         A = @inferred f * Multiplication(f)
@@ -58,6 +63,16 @@ import ApproxFunBase: PointSpace, HeavisideSpace, PiecewiseSegment, dimension, V
             f2 = Fun(Foo(), PointSpace(1:10), 10)
             @test coefficients(f1) == coefficients(f2)
         end
+
+        @testset "union" begin
+            @testset "trivial" begin
+                s = PointSpace(0:3)
+                @test union(s) == s
+                @test union(s, s) == s
+                @test union(s, s, s) == s
+                @test union(s, s, s, s) == s
+            end
+        end
     end
 
     @testset "Derivative operator for HeavisideSpace" begin
@@ -141,4 +156,13 @@ import ApproxFunBase: PointSpace, HeavisideSpace, PiecewiseSegment, dimension, V
             @test (@inferred ApproxFunBase.promotespaces(v)) == v
         end
     end
+
+    @testset "AmbiguousSpace" begin
+        a = PointSpace(1:3)
+        for b in Any[ApproxFunBase.UnsetSpace(), ApproxFunBase.NoSpace()]
+            @test union(a, b) == a
+            @test union(b, a) == a
+            @test union(b, b) == b
+        end
+    end
 end

test/runtests.jl

@@ -66,6 +66,21 @@ end
     @test Segment(1,-1) .^ 2 ≡ Segment(1,0)
     @test Segment(1,2) .^ 2 ≡ Segment(1,4)
     @test sqrt.(Segment(1,2)) ≡ Segment(1,sqrt(2))
+
+    @testset "union" begin
+        a = ApproxFunBase.EmptyDomain()
+        b = ApproxFunBase.AnyDomain()
+
+        @test union(a, a) == a
+        @test union(a, b) == a
+        @test union(b, a) == a
+        @test union(b, b) == b
+
+        @testset for d in Any[a, b]
+            @test union(d, 1..2) == 1..2
+            @test union(1..2, d) == 1..2
+        end
+    end
 end
 
 @time include("MatrixTest.jl")