HPInc / HP-Digital-Microfluidics

HP Digital Microfluidics Software Platform and Libraries
MIT License
3 stars 1 forks source link

Infinite recursion on in DML `_conversion_to()` #276

Closed EvanKirshenbaum closed 8 months ago

EvanKirshenbaum commented 8 months ago

It doesn't happen always, but I'm finding that some of the time, if I bring up an interactive wombat and shift click on a pad to add a drop, if I then call d = clicked pad's drop, I get a RecursionError thrown trying to do a conversion.

The stack at that point is

  File "D:\dmf-git\thylacine\mpam\src\mpam\monitor.py", line 1267, in on_press
    interp.eval_and_print(expr)
  File "D:\dmf-git\thylacine\mpam\src\mpam\interpreter.py", line 50, in eval_and_print
    return self.evaluate(expr).then_call(on_val)
  File "D:\dmf-git\thylacine\mpam\src\mpam\interpreter.py", line 35, in evaluate
    (self.interp.evaluate(expr, cache_as=self.cache_val_as)
  File "D:\dmf-git\thylacine\mpam\src\langsup\dmf_lang.py", line 1313, in evaluate
    executable = compiler.visit(tree)
  File "D:\dmf-git\thylacine\mpam\src\langsup\dmf_lang.py", line 1627, in visit
    return cast(Executable, DMFVisitor.visit(self, tree))
  File "C:\Users\evank\bin\Python\Python310\lib\site-packages\antlr4\tree\Tree.py", line 34, in visit
    return tree.accept(self)
  File "D:\dmf-git\thylacine\mpam\target\generated-sources\antlr4\DMFParser.py", line 1281, in accept
    return visitor.visitExpr_interactive(self)
  File "D:\dmf-git\thylacine\mpam\src\langsup\dmf_lang.py", line 1675, in visitExpr_interactive
    return self.exprAsStatement(ctx.expr())
  File "D:\dmf-git\thylacine\mpam\src\langsup\dmf_lang.py", line 1658, in exprAsStatement
    ex = self.visit(ctx)
  File "D:\dmf-git\thylacine\mpam\src\langsup\dmf_lang.py", line 1627, in visit
    return cast(Executable, DMFVisitor.visit(self, tree))
  File "C:\Users\evank\bin\Python\Python310\lib\site-packages\antlr4\tree\Tree.py", line 34, in visit
    return tree.accept(self)
  File "D:\dmf-git\thylacine\mpam\target\generated-sources\antlr4\DMFParser.py", line 3066, in accept
    return visitor.visitName_assign_expr(self)
  File "D:\dmf-git\thylacine\mpam\src\langsup\dmf_lang.py", line 1743, in visitName_assign_expr
    if var_type < Type.BINARY_STATE:
  File "D:\dmf-git\thylacine\mpam\src\langsup\type_supp.py", line 210, in __lt__
    return self is not rhs and self.can_convert_to(rhs)
  File "D:\dmf-git\thylacine\mpam\src\langsup\type_supp.py", line 461, in can_convert_to
    return self._conversion_to(want) is not SpecialValueConverter.NO_CONVERSION
  File "D:\dmf-git\thylacine\mpam\src\langsup\type_supp.py", line 354, in _conversion_to
    vc = self._find_conversion_to(other)
  File "D:\dmf-git\thylacine\mpam\src\langsup\type_supp.py", line 318, in _find_conversion_to
    check_and_add(c)
  File "D:\dmf-git\thylacine\mpam\src\langsup\type_supp.py", line 293, in check_and_add
    c = compose(to_middle, middle._conversion_to(other))

with the last three lines repeating and the stack ending with


  File "D:\dmf-git\thylacine\mpam\src\langsup\type_supp.py", line 354, in _conversion_to
    vc = self._find_conversion_to(other)
  File "D:\dmf-git\thylacine\mpam\src\langsup\type_supp.py", line 318, in _find_conversion_to
    check_and_add(c)
  File "D:\dmf-git\thylacine\mpam\src\langsup\type_supp.py", line 290, in check_and_add
    to_middle = self._conversion_to(middle)
  File "D:\dmf-git\thylacine\mpam\src\langsup\type_supp.py", line 350, in _conversion_to
    vc = self._conversions.get(other, None)
  File "D:\dmf-git\thylacine\mpam\src\langsup\type_supp.py", line 205, in __hash__
    return id(self)

The problem appears to be the check to see if the type of the expression clicked pad's drop is a binary state:

            if vt is MISSING:
                new_decl = True
                var_type = value.return_type
                if var_type < Type.BINARY_STATE:

but I can't see why that would be a problem. var_type should be maybe drop, and that shouldn't cause any sort of loop.

Migrated from internal repository. Originally created by @EvanKirshenbaum on Jun 09, 2023 at 2:28 PM PDT. Closed on Jun 22, 2023 at 2:28 PM PDT.
EvanKirshenbaum commented 8 months ago

This issue was referenced by the following commit before migration:

EvanKirshenbaum commented 8 months ago

Ran into the same problem when testing out samples (#271). The following should fail:

s = an empty int sample;
s : add(3 ms);

but instead of failing (trying to convert the int sample to a time sample), it was similarly blowing the stack. It turns out that the problem is that a T can be converted to a Maybe(T) ... and vice versa. When this is in the middle of a possible conversion path, we wind up converting a T to an X by converting a Maybe(T) to an X by converting a T to an X by ... and eventually crashing.

The solution I decided upon had two components:

  1. First, _conversion_to() and _find_conversion_to() changed their return types from _Converter to MissingOr[Converter], where a value of MISSING means "I don't know that it won't work eventually, but this question is already on the stack, so there's no point in asking it again.
  2. Second, Type now has a class attribute _in_progress_conversion_searches, which is a set of tuples of from/to types. In _find_conversion_to(), before calling check_and_add() to handle any possible intermediaries, it adds the current pair to it, and when its done, it removes the pair. At the beginning of _find_conversion_to(), if the desired pair is in the set, it returns MISSING, and in check_and_add(), if either call to _conversion_to() returns MISSING, the middle candidate is skipped.

I'm still not sure why the initial problem I was seeing with clicked pad's drop didn't always fail, so it's possible that there was more that was going on there, but I haven't been able to make that infinitely regress since making this change, so I'm going to declare this done for now.

Migrated from internal repository. Originally created by @EvanKirshenbaum on Jun 22, 2023 at 2:28 PM PDT.