Skip to content

Add a funsor.typing module for runtime dispatch and type-checking#451

Merged
eb8680 merged 73 commits intomasterfrom
typing-module-funsormeta
Feb 22, 2021
Merged

Add a funsor.typing module for runtime dispatch and type-checking#451
eb8680 merged 73 commits intomasterfrom
typing-module-funsormeta

Conversation

@eb8680
Copy link
Member

@eb8680 eb8680 commented Feb 2, 2021

Addresses #351, #355. Supercedes #433, #434. Would unblock #423, although it might be easier to fix that independently.

Overview

This PR adds a self-contained funsor.typing module for runtime dispatch and typechecking of parametric types, especially Funsor term types.

It contains three distinct pieces of functionality extracted from FunsorMeta:

  1. Replacements for type, isinstance and issubclass that are compatible with a limited subset of Funsor/typing-style generic types, including Funsor terms, typing.Tuples and typing.FrozenSets, mimicking similar APIs in pytypes (albeit with much less coverage of typing).
  2. A new metaclass funsor.typing.GenericTypeMeta whose purpose is to support parametric subtyping, handle cons-hashing of parametric types and support runtime issubclass checks, and a minimal introspection API matching typing's get_args/get_origin/get_type_hints.
  3. A multipledispatch.dispatcher.Dispatcher subclass that is capable of making use of this new functionality and is used by all Funsor dispatched_interpretations via funsor.registry.KeyedRegistry

I have also refactored and drastically simplified FunsorMeta using this machinery. In a followup PR I will do the same for funsor.domains.ArrayType.

Context

After these changes, it should be possible to use some of the Python standard typing library directly when describing funsor input and output types or when writing patterns, e.g. we could simply replace Product from #430 with typing.Tuple:

class Tuple(Funsor):
    def __init__(self, args):
        ...
        output = typing.Tuple[tuple(arg.output for arg in args)]
        ...

...or use typing.Tuple, including variadic cases, in patterns like the ones needed for Finitary in #423:

@eager.register(Finitary)
def eager_finitary_generic(
        op: EinsumOp, 
        args: typing.Tuple[typing.Union[Tensor, Number], ...]  # variadic
    ) -> typing.Union[Tensor, Number]:
    ...

As suggested in #355, we could even replace many of the assertions in our Funsor.__init__ methods with runtime type checking in interpreters, eliminating lots of boilerplate code and potentially boosting performance (by making many assertions optional):

class FunsorMeta(type):
    def __call__(cls, *args, **kwargs):
        ...
        if interpreter.TYPECHECK:
            assert all(isinstance(arg, static_arg_type)
                        for arg, static_arg_type in zip(args, get_type_hints(cls.__init__)))
        return interpret(cls, *args)
# example
class Reduce(Funsor):
-     def __init__(self, op, arg, reduced_vars):
+     def __init__(self, op: ops.AssociativeOp, arg: Funsor, reduced_vars: typing.FrozenSet[Variable]):
-         assert callable(op)
-         assert isinstance(arg, Funsor)
-         assert isinstance(reduced_vars, frozenset)
-         assert all(isinstance(v, Variable) for v in reduced_vars)
          reduced_names = frozenset(v.name for v in reduced_vars)
          ...

In #355 I had originally suggested going even further toward integrating typing with Funsor interpretations than the changes here by making parametric Funsor subtypes use typing.Generic. I am somewhat skeptical about taking that additional step because of how difficult it is to interact with typing.Generic at runtime, and have chosen in this PR to retain a simplified version of the GenericTypeMeta class as a substrate for Funsor and FunsorMeta.

Tested

  • Exercised by all existing tests that use interpretations

Remaining tasks

  • Resolve any typing-related dependency issues for Python 3.6, 3.7, 3.8, 3.9
  • Move the existing issubclass tests in test_terms.py to a new file test_typing.py
  • Add more standalone tests to test_typing.py
  • Add more documentation

Triaged

  • Investigate performance regressions with the new Dispatcher and FunsorMeta implementations - this appears to be less severe than originally feared, so I am OK with addressing performance in future PRs unless there's a really easy fix.

Copy link
Member

@fritzo fritzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the detailed PR description. Tests look great, and interface looks standard. Glad to hear the performance regression is minor.

@fritzo
Copy link
Member

fritzo commented Feb 21, 2021

nit: could you add a funsor.typing section to the docs, and ensure docs are correctly generated?

@eb8680
Copy link
Member Author

eb8680 commented Feb 22, 2021

nit: could you add a funsor.typing section to the docs, and ensure docs are correctly generated?

Docs now seem to render correctly for me locally.

Copy link
Member

@fritzo fritzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM feel free to merge, then I'll update #463

@eb8680 eb8680 merged commit a5b3556 into master Feb 22, 2021
@eb8680 eb8680 deleted the typing-module-funsormeta branch February 22, 2021 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants