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

Make interpreters compatible with Python type-checking #355

Open
eb8680 opened this issue Aug 17, 2020 · 2 comments
Open

Make interpreters compatible with Python type-checking #355

eb8680 opened this issue Aug 17, 2020 · 2 comments
Labels

Comments

@eb8680
Copy link
Member

eb8680 commented Aug 17, 2020

Currently, while Funsor makes heavy use of multipledispatch on parametric types, interpretations and Funsor terms are not compatible with static type-checkers like MyPy or even runtime type-checkers like pytypes. It would improve the development experience, increase performance and beneficially constrain Funsor's design to make Funsor interpreters and lazy expressions more compatible with Python's standard typing module and even with MyPy.

At a high level, this seems straightforward: just make all subclasses of funsor.Funsor inherit from typing.Generic and move type annotations from register calls to function signatures, e.g.:

Op = TypeVar("Op", bound=ops.AssociativeOp)
Arg = TypeVar("Arg", bound=Funsor)
ReducedVars = TypeVar("ReducedVars", bound=FrozenSet[str])

class Reduce(Funsor, Generic[Op, Arg, ReducedVars]):
    def __init__(self, op: Op, arg: Arg, reduced_vars: ReducedVars):
        ...  # note we should be able to remove all hand-written type assertions here

Lhs = TypeVar("Lhs", bound=Funsor)
Rhs = TypeVar("Rhs", bound=Funsor)

class Binary(Funsor, Generic[Op, Lhs, Rhs]):
    def __init__(self, op, lhs, rhs):
        ...

# a random example pattern (a+b).sum() -> (a.sum() + b.sum())
@eager.register(Reduce)
def eager_reduce_example_pattern(
        op: ops.AddOp, 
        arg: Binary[ops.AddOp, Funsor, Funsor],
        reduced_vars: FrozenSet[str]
    ):
    return Binary(arg.op, Reduce(op, arg.lhs, reduced_vars), Reduce(op, arg.rhs, reduced_vars))

We'd also have to modify KeyedRegistry along the lines of mrocklin/multipledispatch#69 using pytypes.deep_type for dispatch to continue to work correctly, and define FunsorMeta.__subclasscheck__ with pytypes.is_subtype rather than the custom logic it currently contains.

Unfortunately, there are lots of design details that may prevent fully statically checkable interpreters or at least make them undesirable for other reasons. For example, the obtrusive type variables in the snippet would seem to be mandatory - if their creation were automated and folded into FunsorMeta.__init__, the result would not be compatible with static type checkers like MyPy, which seems to be incompatible with any internal logic in metaclasses. It's also not clear that the way we construct finer types at runtime with reflect between intepreter calls would be compatible with MyPy.

These caveats may not apply to runtime type checkers like pytypes, which could still cut out lots of boilerplate and improve performance, so perhaps this is the best starting point. Note that there are Python version compatibility issues upstream in pytypes that might cause other problems (but seem to be mostly resolved in the master branch).

Note that this issue is orthogonal to #351, which is about the types of Funsors themselves rather than the type signatures of interpretations, despite the similar motivations.

@eb8680
Copy link
Member Author

eb8680 commented Aug 20, 2020

I spent some time on this today (see branch), but I'm not convinced yet that it will actually work... As one might expect, typing is really not cut out for this sort of thing. I also had to modify multipledispatch.

@eb8680
Copy link
Member Author

eb8680 commented Feb 22, 2021

Most of the infrastructure necessary for this was added in #451. The remaining work is moving patterns into type hints where possible and checking that the new funsor.typing.GenericTypeMeta from #451 can be made to work in at least a very limited capacity with mypy, and perhaps adding a mypy stage to CI.

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

No branches or pull requests

1 participant