Brandon T. Willard

Results 434 comments of Brandon T. Willard

> @brandonwillard can you please share an example of a large `Scan` graph with which I can replicate this issue locally? I would like to attempt to solve this bug....

There's only explicit dispatch constructor/`_reify` support for `tuple`, and since `NamedTuple`'s type is a subclass of `tuple`, it's being interpreted as the latter. We could attempt to add broader support...

`unification.core._reify` is the dispatch function underlying `unification.core.reify`. It needs to be overloaded in order to add support for new types.

This looks like an "occurs-check"-related issue appearing in `reify`, because the unification state would be `{q: (q,)}` and `reify` would attempt to walk `(q,)` and replace the `q` inside it...

FYI: I labeled this a bug for the time being, but, since `unification` doesn't advertise an "occurs-check" feature/support, it's more of a feature request.

This looks great, thanks! Will review shortly.

> I tried to reproduce the bug locally but wasn't successful in reproducing it. I'm able to reproduce the error in CI locally from `test_Subtensor` on this branch. Try clearing...

> We need to discuss more on this approach and see how the functions are pickled in `dill`. The test failure on CI is very different from what it is...

> Hmm, this gets tricker when we start digging at a low level. IMHO, we can do the following: > > 1. Use the previous approach of moving static functions...

> I'll take a look at this @brandonwillard That would be great, thanks!