"generalize" definition of is_same_except and == for MLJType

Author: ablaom

src/MLJModelInterface.jl

@@ -23,7 +23,8 @@ const MODEL_TRAITS = [
     :hyperparameter_types,
     :hyperparameter_ranges,
     :iteration_parameter,
-    :supports_training_losses]
+    :supports_training_losses,
+    :deep_properties]
 
 # ------------------------------------------------------------------------
 # Dependencies (ScientificTypes and StatisticalTraits have none)

src/equality.jl

@@ -1,45 +1,116 @@
+function _equal_to_depth_one(x1, x2)
+    names = propertynames(x1)
+    names === propertynames(x2) || return false
+    for name in names
+            getproperty(x1, name) == getproperty(x2, name) || return false
+    end
+    return true
+end
+
+@doc """
+    deep_properties(::Type{<:MLJType})
+
+Given an `MLJType` subtype `M`, the value of this trait should be a
+tuple of any properties of `M` to be regarded as "deep".
+
+When two instances of type `M` are to be tested for equality, in the
+sense of `==` or `is_same_except`, then the values of a "deep"
+property (whose values are assumed to be of composite type) are deemed
+to agree if all corresponding properties *of those property values*
+are `==`.
+
+Any property of `M` whose values are themselves of `MLJType` are
+"deep" automatically, and should not be included in the trait return
+value.
+
+See also [`is_same_except`](@ref)
+
+### Example
+
+Consider an `MLJType` subtype `Foo`, with a single field of
+type `Bar` which is *not* a subtype of `MLJType`:
+
+    mutable struct Bar
+        x::Int
+    end
+
+    mutable struct Foo <: MLJType
+        bar::Bar
+    end
+
+Then the mutability of `Foo` implies `Foo(1) != Foo(1)` and so, by the
+definition `==` for `MLJType` objects (see [`is_same_except`](@ref))
+we have
+
+    Bar(Foo(1)) != Bar(Foo(1))
+
+However after the declaration
+
+    MLJModelInterface.deep_properties(::Type{<:Foo}) = (:bar,)
+
+We have
+
+    Bar(Foo(1)) == Bar(Foo(1))
+
 """
-    is_same_except(m1::MLJType, m2::MLJType, exceptions::Symbol...)
+StatisticalTraits.deep_properties
+
+
+"""
+    is_same_except(m1, m2, exceptions::Symbol...; deep_properties=Symbol[])
+
+If both `m1` and `m2` are of `MLJType`, return `true` if the
+following conditions all hold, and `false` otherwise:
 
-Returns `true` only the following conditions all hold:
+- `typeof(m1) === typeof(m2)`
 
-- `m1` and `m2` have the same type.
+- `propertynames(m1) === propertynames(m2)`
 
-- `m1` and `m2` have the same undefined fields.
+- with the exception of properties listed as `exceptions` or bound to
+  an `AbstractRNG`, each pair of corresponding property values is
+  either "equal" or  both undefined.
 
-- Corresponding fields agree, or are listed as `exceptions`, or have
-  `AbstractRNG` as values (one or both)
+The meaining of "equal" depends on the type of the property value:
 
-Here "agree" is in the sense of "==", unless the objects are themselves of
-`MLJType`, in which case agreement is in the sense of `is_same_except` with
-no exceptions allowed.
+- values that are themselves of `MLJType` are "equal" if they are
+equal in the sense of `is_same_except` with no exceptions.
+
+- values that are not of `MLJType` are "equal" if they are `==`.
+
+In the special case of a "deep" property, "equal" has a different
+meaning; see [`deep_properties`](@ref)) for details.
+
+If `m1` or `m2` are not `MLJType` objects, then return `==(m1, m2)`.
 
-Note that Base.== is overloaded such that `m1 == m2` if and only if
-`is_same_except(m1, m2)`.
 """
 is_same_except(x1, x2) = ==(x1, x2)
-function is_same_except(m1::M1, m2::M2,
-            exceptions::Symbol...) where {M1<:MLJType,M2<:MLJType}
-    if typeof(m1) != typeof(m2)
-        return false
-    end
-    defined1 = filter(fieldnames(M1)|>collect) do fld
-        isdefined(m1, fld) && !(fld in exceptions)
-    end
-    defined2 = filter(fieldnames(M1)|>collect) do fld
-        isdefined(m2, fld) && !(fld in exceptions)
-    end
-    if defined1 != defined2
-        return false
-    end
-    same_values = true
-    for fld in defined1
-        same_values = same_values &&
-            (is_same_except(getfield(m1, fld), getfield(m2, fld)) ||
-             getfield(m1, fld) isa AbstractRNG ||
-             getfield(m2, fld) isa AbstractRNG)
+function is_same_except(m1::M1,
+                        m2::M2,
+                        exceptions::Symbol...) where {M1<:MLJType,M2<:MLJType}
+    typeof(m1) === typeof(m2) || return false
+    names = propertynames(m1)
+    propertynames(m2) === names || return false
+
+    for name in names
+        if !(name in exceptions)
+            if !isdefined(m1, name)
+               !isdefined(m2, name) || return false
+            elseif isdefined(m2, name)
+                if name in deep_properties(M1)
+                    _equal_to_depth_one(getproperty(m1,name),
+                                        getproperty(m2, name)) || return false
+                else
+                    (is_same_except(getproperty(m1, name),
+                                    getproperty(m2, name)) ||
+                     getproperty(m1, name) isa AbstractRNG ||
+                     getproperty(m2, name) isa AbstractRNG) || return false
+                end
+            else
+                return false
+            end
+        end
     end
-    return same_values
+    return true
 end
 
 ==(m1::M1, m2::M2) where {M1<:MLJType,M2<:MLJType} = is_same_except(m1, m2)

test/equality.jl

@@ -8,10 +8,24 @@ mutable struct Foo <: MLJType
     y::Int
 end
 
-mutable struct Bar <: MLJType
+mutable struct Bar{names} <: MLJType
     rng::AbstractRNG
-    x::Int
-    y::Int
+    v::Tuple{Int,Int}
+    Bar{names}(rng, x, y) where names =
+        new{names}(rng, (x, y))
+end
+
+Bar(rng, x, y) = Bar{(:x, :y)}(rng, x, y)
+
+# overload `getproperty` so that components of `v` are accessed with
+# the names given in `names` (which will be (:x, :y) when using
+# the above constructor):
+Base.propertynames(::Bar{names}) where names = (:rng, names...)
+function Base.getproperty(b::Bar{names}, name::Symbol) where names
+    name === :rng && return getfield(b, :rng)
+    v = getfield(b, :v)
+    name === names[1] && return v[1]
+    return v[2]
 end
 
 mutable struct Super <: MLJType
@@ -25,6 +39,35 @@ mutable struct Partial <: MLJType
     Partial(x) = new(x)
 end
 
+mutable struct Sub
+    x::Int
+end
+
+mutable struct Deep
+    x::Int
+    s::Union{Sub,Int}
+end
+
+mutable struct Super2 <: MLJType
+    sub::Sub
+    z::Int
+end
+
+MLJModelInterface.deep_properties(::Type{<:Super2}) = (:sub,)
+
+@testset "_equal_to_depth_one" begin
+    d1 = Deep(1, 2)
+    d2 = Deep(1, 2)
+    @test MLJModelInterface._equal_to_depth_one(d1, d2)
+    d2.x = 3
+    @test !MLJModelInterface._equal_to_depth_one(d1, d2)
+
+    d1 = Deep(1, Sub(2))
+    d2 = Deep(1, Sub(2))
+    @test !MLJModelInterface._equal_to_depth_one(d1, d2)
+end
+
+
 @testset "equality for MLJType" begin
     f1 = Foo(MersenneTwister(7), 1, 2)
     f2 = Foo(MersenneTwister(8), 1, 2)
@@ -56,9 +99,15 @@ end
 
     p1 = Partial(1)
     p2 = Partial(1)
+    @test p1 == p2
     p2.y = [1,2]
     @test !(p1 == p2)
 
+    # test of "deep" properties
+    s1 = Super2(Sub(1), 2)
+    s2 = Super2(Sub(1), 2)
+    @test s1 == s2
+
 end
 
 @testset "in(x, collection) for MLJType" begin