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

Figure out a consistent and robust way of defining and testing interfaces #2434

Open
Tracked by #2420
penelopeysm opened this issue Dec 7, 2024 · 2 comments
Open
Tracked by #2420

Comments

@penelopeysm
Copy link
Member

We have a lot of interfaces across the Turing ecosystem. Few of them are formalised or testable. This is partly a result of Julia's multiple dispatch mechanism: it's possible to define InterfacePackage.foo() with whatever signature we like, and then overload it in ImplementationPackage with a completely different signature. Thus, when implementing a function, there's actually zero need to pay attention to what the interface demands.

The problem with this is that:

  1. The interface code becomes useless. We pretend to maintain it but in reality there is no cost to not maintaining it.
  2. Because the interface is not necessarily obeyed, it's impossible to figure out how the implementations work except by reading the source code.

There aren't really any amazing solutions to this in Julia. We should try to figure out what the least bad solution is.

@yebai
Copy link
Member

yebai commented Dec 9, 2024

It might be helpful: https://github.com/rafaqz/Interfaces.jl, but I haven't used it myself, so I'm not sure how mature it is.

@penelopeysm
Copy link
Member Author

penelopeysm commented Dec 10, 2024

Indeed, Will brought this up before too and I've seen it mentioned once or twice on Discourse.

I've read through the Interfaces.jl docs a couple of times now, and my assessment is that it's almost completely syntax sugar, in the sense that Interfaces.jl doesn't let you do very much that you can't already do by writing a standard test.*

As a demonstration, here's a simplified version of the example in the Interfaces.jl docs:

#################
# src/animal.jl #
#################

abstract type Animal end

function age end
function walk end

components = (
    mandatory = (
        age = (
            "all animals have a `Real` age" => x -> age(x) isa Real,
        ),
    ),
    optional = (
        walk = "this animal can walk" => x -> walk(x) isa String,
    ),
)

description = """
Defines a generic interface for animals to do the things they do best.
"""

@interface AnimalInterface Animal components description

struct Duck <: Animal
    age::Int
end

age(duck::Duck) = duck.age
walk(::Duck) = "waddle"
@implements AnimalInterface{(:walk,)} Duck [Duck(1), Duck(2)]

##################
# test/animal.jl #
##################
Interfaces.test(AnimalInterface)

This is syntax sugar for the following:

#################
# src/animal.jl #
#################

abstract type Animal end

function age end
function walk end

"""
Test that animals do the things they do best.
"""
function test_animal_interface(a::Animal, optional::Tuple{Symbol}=())
    test_age(a)
    :walk in optional && test_walk(a)
end
test_age(a::Animal) = @test age(a) isa Real
test_walk(a::Animal) = @test walk(a) isa String

struct Duck <: Animal
    age::Int
end

age(duck::Duck) = duck.age
walk(::Duck) = "waddle"

##################
# test/animal.jl #
##################
@testset for duck in [Duck(1), Duck(2)]
    test_animal_interface(duck, (:walk,))
end

I find the latter more readable because there is one less layer of indirection. You don't need to go and read up about how an additional package works.

Also, if you want to modify the behaviour of the tests, you also aren't constrained by the limitations of the package. For example, Interfaces.jl's interface gets a little bit ugly when you want to use test functions which take multiple arguments. You have to pass in an Arguments type rather than just having a normal n-ary function. This means more glue code (consider e.g. the case where we might have functions to test things in DynamicPPL, but you also want to use those testing functions in Turing).

Lastly, sometimes there are interfaces that aren't based on abstract types. The example I have off the top of my head are the demo models in DynamicPPL, which all have type Model{typeof(function_name)}, and we can't declare those to be subtypes of an abstract type. Nevertheless, they still do have their own interface that can be used by other tests. I don't know how or whether Interfaces.jl handles this.


* There is indeed one nice thing that Interfaces.jl lets you do, which is the @implements macro that automatically registers an instance of a subtype so that it can be automatically tested later. Importantly, that declaration can be put in the source code next to the definition of the type so it's easier to not forget to do it. The alternative to this is to manually 'remember' which things need to be tested, and that information would be located in the test suite, so is further away from the definition. I personally don't consider this a big enough win to use it, though.


tl;dr

Having surveyed a few corners of Julia land, I kinda believe the best way is still to just stick to basic, understandable testing functions.

Also, for any abstract type we should have a minimal test double as demonstrated in the Invenia interfaces blog post.

And finally I think abstract types, their interfaces, and their test doubles should always live in a single file with nothing else in them. Basically, don't mix interfaces and implementations in the same file. Julia lets us split files up as you like, so we should make use of that to make the code as clear as possible.

I think the best place to start with this is AbstractMCMC.AbstractSampler and/or AbstractMCMC.AbstractChains. The latter is used in a number of DynamicPPL functions (presumably with the intention of avoiding a strong MCMCChains dependency) but afaik the interface is not very clearly defined and it wouldn't surprise me if DynamicPPL accidentally used functionality that is only present in MCMCChains.Chains.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants