From: Tim Vieira <tim.f.vieira@gmail.com>
Date: Wed, 3 Jul 2013 20:57:56 +0000 (-0400)
Subject: Overhaul error message displays
X-Git-Url: https://hydra-www.ietfng.org/gitweb/?a=commitdiff_plain;h=64cb33934f572cd49a9de2cceff5cc8b935de83f;p=dyna2

Overhaul error message displays

 - "pop-time": aggregation errors (e.g. two distinct values following into an
   `=` aggregator) are group by functor/arity, then by type and only 10
   instances are shown.

 - "push-time": errors are grouped by rule, then error type (e.g. TypeError,
   DivisionByZero), only 10 instances of the error are displayed.

 - These changes help avoid an overwhelming cascade of very similar
   errors. Users should be able to trace the sources of these errors using other
   tools (e.g. `trace`)

Resolve issue #36

Program rules available in $rule -- doesn't do anything yet however.

Tweaks to doctest runner.
---

diff --git a/examples/errors.dyna b/examples/errors.dyna
index 084a021..aa39436 100644
--- a/examples/errors.dyna
+++ b/examples/errors.dyna
@@ -15,3 +15,12 @@ e := 0.
 
 d += null.
 d += 1.
+
+
+a(X) := f(X,Y).
+
+f(1,1) := 1.
+f(1,2) := 2.
+
+f(2,1) := 1.
+f(2,2) := 2.
diff --git a/src/Dyna/Backend/Python/defn.py b/src/Dyna/Backend/Python/defn.py
index a0a3356..87fd23c 100644
--- a/src/Dyna/Backend/Python/defn.py
+++ b/src/Dyna/Backend/Python/defn.py
@@ -7,7 +7,7 @@ from __future__ import division
 
 import operator
 from collections import Counter
-from utils import drepr, _repr
+from utils import drepr, _repr, user_vars
 from errors import AggregatorError
 
 """
@@ -43,36 +43,43 @@ class Aggregator(object):
 class BAggregator(Counter, Aggregator):
 #    def __init__(self):
 #        super(BAggregator, self).__init__()
-    def inc(self, val, ruleix, variables):
+    def inc(self, val, _ruleix, _variables):
         self[val] += 1
-    def dec(self, val, ruleix, variables):
+    def dec(self, val, _ruleix, _variables):
         self[val] -= 1
     def fromkeys(self, *_):
         assert False, "This method should never be called."
 
 
-class PlusEquals(object):
-    __slots__ = 'pos', 'neg'
-    def __init__(self):
-        self.pos = 0
-        self.neg = 0
-    def inc(self, val, ruleix, variables):
-        self.pos += val
-    def dec(self, val, ruleix, variables):
-        self.neg += val
-    def fold(self):
-        return self.pos - self.neg
+#class PlusEquals(object):
+#    __slots__ = 'pos', 'neg'
+#    def __init__(self):
+#        self.pos = 0
+#        self.neg = 0
+#    def inc(self, val, ruleix, variables):
+#        self.pos += val
+#    def dec(self, val, ruleix, variables):
+#        self.neg += val
+#    def fold(self):
+#        return self.pos - self.neg
 
 
 class ColonEquals(BAggregator):
-    def inc(self, val, ruleix, variables):
+    def inc(self, val, ruleix, _variables):
         self[ruleix, val] += 1
-    def dec(self, val, ruleix, variables):
+    def dec(self, val, ruleix, _variables):
         self[ruleix, val] -= 1
     def fold(self):
-        vs = [v for v, cnt in self.iteritems() if cnt > 0]
+        vs = [v for v, m in self.iteritems() if m > 0]
         if vs:
-            return max(vs)[1]
+            [i, v] = max(vs)
+            vs = {v for (r, v) in vs if r == i}   # filter down to max rule index
+            if len(vs) == 1:
+                return v
+            else:
+                vs = list(vs)   # for stability
+                vs.sort()
+                raise AggregatorError('`:=` got conflicting values %s for rule index %s' % (vs, i))
 
 
 class Equals(BAggregator):
@@ -82,19 +89,15 @@ class Equals(BAggregator):
         self[val] -= 1
     def fold(self):
         vs = [v for v, cnt in self.iteritems() if cnt > 0]
-        if len(vs) != 1:
+        if len(vs) == 0:
+            return
+        if len(vs) == 1:
+            return vs[0]
+        else:
             vs.sort()   # for stability
             raise AggregatorError('`=` got conflicting values %s' % (vs,))
-        return vs[0]
 
 
-def user_vars(variables):
-    "Post process the variables past to emit (which passes them to aggregator)."
-    # remove the 'u' prefix on user variables 'uX'
-    # Note: We also ignore user variables with an underscore prefix
-    return tuple((name[1:], val) for name, val in variables
-                 if name.startswith('u') and not name.startswith('u_'))
-
 
 from collections import namedtuple
 class Result(namedtuple('Result', 'value variables')):
@@ -104,13 +107,12 @@ class Result(namedtuple('Result', 'value variables')):
 
 class DictEquals(BAggregator):
 
-    def inc(self, val, ruleix, variables):
-        # I think we only want user variables -- XXX: are we guaranteed to have
-        # all of the user variables?
+    def inc(self, val, _ruleix, variables):
+        # I think we only want user variables
         vs = user_vars(variables)
         self[val, vs] += 1
 
-    def dec(self, val, ruleix, variables):
+    def dec(self, val, _ruleix, variables):
         vs = user_vars(variables)
         self[val, vs] -= 1
 
@@ -200,15 +202,15 @@ class b_or_equals(BAggregator):
 
 class set_equals(BAggregator):
     def fold(self):
-        from stdlib import todynalist
+        from stdlib import todyna
         s = {x for x, m in self.iteritems() if m > 0}
         if len(s):
-            return todynalist(s)
+            return todyna(s)
 
 class bag_equals(BAggregator):
     def fold(self):
-        from stdlib import todynalist
-        return todynalist(Counter(self).elements())
+        from stdlib import todyna
+        return todyna(list(Counter(self).elements()))
 
 
 # map names to functions
diff --git a/src/Dyna/Backend/Python/dyna-doctest.py b/src/Dyna/Backend/Python/dyna-doctest.py
index 3d46866..9be0c3f 100755
--- a/src/Dyna/Backend/Python/dyna-doctest.py
+++ b/src/Dyna/Backend/Python/dyna-doctest.py
@@ -7,11 +7,11 @@ from interpreter import Interpreter
 from repl import REPL
 from cStringIO import StringIO
 
-from utils import red, green, strip_comments
+from utils import red, green, yellow, strip_comments
 
 
 def extract(code):
-    for block in re.compile('^:- ', re.MULTILINE).split(code):
+    for block in re.compile('^> ', re.MULTILINE).split(code):
         for cmd, expect in re.findall('(.*?)\n([\w\W]*)$', block):
             yield cmd, expect
 
@@ -19,12 +19,12 @@ def extract(code):
 def run(code):
     interp = Interpreter()
     repl = REPL(interp)
-
+    errors = []
     for cmd, expect in extract(code):
-        if not cmd.strip():
+        if not strip_comments(cmd).strip():
             print
             continue
-        print ':-', cmd
+        print yellow % '> %s' % cmd
         sys.stdout = x = StringIO()
         try:
             repl.onecmd(cmd)
@@ -35,10 +35,16 @@ def run(code):
         if strip_comments(expect) != strip_comments(got):
             print green % expect
             print red % got
+            errors.append(cmd, expect, got)
         else:
             print x.getvalue().rstrip()
         print
 
+    if not errors:
+        print green % 'PASS!'
+    else:
+        print red % '%s errors' % len(errors)
+    print
 
 if __name__ == '__main__':
     for filename in sys.argv[1:]:
diff --git a/src/Dyna/Backend/Python/interpreter.py b/src/Dyna/Backend/Python/interpreter.py
index 50f1824..6c0cc9b 100644
--- a/src/Dyna/Backend/Python/interpreter.py
+++ b/src/Dyna/Backend/Python/interpreter.py
@@ -122,7 +122,7 @@ from utils import ip, red, green, blue, magenta, yellow, parse_attrs, \
 from prioritydict import prioritydict
 from config import dotdynadir
 from errors import crash_handler, DynaInitializerException, AggregatorError, DynaCompilerError
-
+from stdlib import todyna
 
 class Rule(object):
     def __init__(self, idx):
@@ -235,6 +235,10 @@ class Interpreter(object):
         if nullary:
             print >> out
         for x in others:
+
+            if x.startswith('$rule/'):
+                continue
+
             y = str(self.chart[x])   # skip empty chart
             if y:
                 print >> out, y
@@ -249,12 +253,52 @@ class Interpreter(object):
         print >> out
         print >> out, 'Errors'
         print >> out, '======'
+
+        I = defaultdict(lambda: defaultdict(list))
+        E = defaultdict(lambda: defaultdict(list))
         for item, (val, es) in self.error.items():
-            print >> out,  'because %r is %s:' % (item, _repr(val))
             for e, h in es:
-                if h is not None:
-                    r = h.rule
-                    print >> out, '    %s\n        in rule %s\n            %s' % (e, r.span, r.src)
+                if h is None:
+                    I[item.fn][type(e)].append((e, item, val))
+                else:
+                    E[h.rule][type(e)].append((e, item, val))
+
+        # aggregation errors
+        for r in I:
+            print >> out, 'Error(s) aggregating %s:' % r
+            for etype in I[r]:
+                print >> out, '  %s:' % etype.__name__
+                for i, (e, item, value) in enumerate(sorted(I[r][etype])):                       # todo: probably don't want to show ten million errors
+                    if i >= 5:
+                        print >> out, '    %s more ...' % (len(I[r][etype]) - i)
+                        break
+                    print >> out, '    when `%s` = %s' % (item, _repr(value))
+                    print >> out, '      %s' % (e)
+                print >> out
+
+        # errors pertaining to rules
+        for r in E:
+            print >> out, 'Error(s) in rule:', r.span
+            print >> out
+            for line in r.src.split('\n'):
+                print >> out, '   ', line
+            print >> out
+            for etype in E[r]:
+                print >> out, '  %s:' % etype.__name__
+                for i, (e, item, value) in enumerate(sorted(E[r][etype])):                       # todo: probably don't want to show ten million errors
+                    if i >= 5:
+                        print >> out, '    %s more ...' % (len(E[r][etype]) - i)
+                        break
+                    print >> out, '    when `%s` = %s' % (item, _repr(value))
+                    print >> out, '      %s' % (e)
+                print >> out
+
+#        for item, (val, es) in self.error.items():
+#            print >> out,  'because %r is %s:' % (item, _repr(val))
+#            for e, h in es:
+#                if h is not None:
+#                    r = h.rule
+#                    print >> out, '    %s\n        in rule %s\n            %s' % (e, r.span, r.src)
         print >> out
 
     def dump_rules(self):
@@ -300,6 +344,11 @@ class Interpreter(object):
         except KeyError:
             print 'Rule %s not found.' % idx
             return
+
+        # remove $rule
+        if hasattr(rule, 'item'):
+            self.delete_emit(rule.item, True, ruleix=None, variables=None)
+
         # Step 1: remove update handlers
         for u in rule.updaters:
             for xs in self.updaters.values():
@@ -309,10 +358,12 @@ class Interpreter(object):
         # Step 2: run initializer in delete mode
         if rule.init is not None:
             rule.init(emit=self.delete_emit)
-            # Step 3; go!
-            return self.go()
         # TODO: probably have to blast any memos from BC computations
 
+        # Step 3; go!
+        return self.go()
+
+
     def go(self):
         try:
             return self._go()
@@ -334,7 +385,7 @@ class Interpreter(object):
                 now = item.aggregator.fold()
 
             except AggregatorError as e:
-                error[item] = ('failed to aggregate item `%r` because %s' % (item, e), [(e, None)])
+                error[item] = (None, [(e, None)])
 
                 now = self.build('$error/0')   # XXX: should go an agenda or run delete?
                 changed[item] = now
@@ -342,7 +393,7 @@ class Interpreter(object):
                 continue
 
             except (ZeroDivisionError, TypeError, KeyboardInterrupt, NotImplementedError) as e:
-                error[item] = ('failed to aggregate %r' % item.aggregator, [(e, None)])
+                error[item] = (None, [(e, None)])
 
                 now = self.build('$error/0')   # XXX: should go an agenda or run delete?
                 changed[item] = now
@@ -481,8 +532,6 @@ class Interpreter(object):
         """
         assert os.path.exists(filename)
 
-
-
         env = imp.load_source('dynamically_loaded_module', filename)
 
         if path(filename + '.anf').exists():       # XXX: should have codegen provide this in plan.py
@@ -536,6 +585,21 @@ class Interpreter(object):
             for e in emits:
                 self.emit(*e, delete=False)
 
+        # ------ $rule for fun and profit -------
+        interp = self
+        def rule(ix, *a):
+            fn = '$rule/%s' % (len(a) + 1)
+            if interp.agg_name[fn] is None:
+                interp.new_fn(fn, ':=')
+            item = interp.build(fn, ix, *a)
+            interp.emit(item, True, ruleix=None, variables=None, delete=False)
+            return item
+        for i in new_rules:
+            r = self.rules[i]
+            agg, head, evals, unifs, result = r.anf[2:]
+            r.item = rule(i, r.src, todyna([head, agg, result, evals, unifs]), r.init, r.query)
+        #-----------------------------------------
+
         return self.go()
 
     def dynac(self, filename):
diff --git a/src/Dyna/Backend/Python/load/sexpr.py b/src/Dyna/Backend/Python/load/sexpr.py
index 1653b2d..358e9b8 100644
--- a/src/Dyna/Backend/Python/load/sexpr.py
+++ b/src/Dyna/Backend/Python/load/sexpr.py
@@ -1,6 +1,6 @@
 from cStringIO import StringIO
 from utils import parse_sexpr
-from stdlib import todynalist
+from stdlib import todyna
 
 
 class sexpr(object):
@@ -33,20 +33,14 @@ class sexpr(object):
         def obj(*a):
             fn = '%s/%s' % (name, len(a))
             if interp.agg_name[fn] is None:
-                interp.new_fn(fn, ':=')
+                interp.new_fn(fn, '=')
             return interp.build(fn, *a)
 
-        def t(xs):
-            if isinstance(xs, basestring):
-                return xs
-            else:
-                return todynalist([t(x) for x in xs])
-
         contents = file(filename).read()
 
         for i, x in enumerate(parse_sexpr(contents)):
             interp.emit(obj(i),
-                        t(x),
+                        todyna(x),
                         ruleix=None,
                         variables=None,
                         delete=False)
diff --git a/src/Dyna/Backend/Python/post/draw_circuit.py b/src/Dyna/Backend/Python/post/draw_circuit.py
index add8c47..6509e5b 100644
--- a/src/Dyna/Backend/Python/post/draw_circuit.py
+++ b/src/Dyna/Backend/Python/post/draw_circuit.py
@@ -28,7 +28,10 @@ def infer_edges(interp):
 
     for r in interp.rules.values():
         if r.init is not None:
-            r.init(emit=_emit)
+            try:
+                r.init(emit=_emit)
+            except (TypeError, ValueError, AssertionError, ZeroDivisionError):
+                pass
         else:
             assert r.query is not None
 
diff --git a/src/Dyna/Backend/Python/post/trace.py b/src/Dyna/Backend/Python/post/trace.py
index 302424b..67be4cc 100644
--- a/src/Dyna/Backend/Python/post/trace.py
+++ b/src/Dyna/Backend/Python/post/trace.py
@@ -89,8 +89,7 @@ def dig(head, visited, tail, groups, interp):
 
     return ['%s = %s' % (yellow % head, cyan % _repr(head.value))] \
         + ['|'] \
-        + branch(contribs) #\
-#        + ['']
+        + branch(contribs)
 
 
 def branch(xs):
@@ -135,9 +134,9 @@ class Crux(object):
     def format(self):
         rule = self.rule
         #src = rule.src.replace('\n',' ').strip()
-        graph = self.graph
         #user_vars = dict(defn.user_vars(self.vs.items()))
 
+        graph = self.graph
         side = [self.get_function(x) for x in graph.outputs if x != rule.anf.result and x != rule.anf.head]
 
         explode = ('%s %s %s' % (self.get_function(rule.anf.head)[1:],  # drop quote on head
@@ -159,7 +158,6 @@ class Crux(object):
 
         return lines
 
-
     def get_function(self, x):
         """
         String of symbolic representation of ``x``, a variable or function, in
diff --git a/src/Dyna/Backend/Python/repl.py b/src/Dyna/Backend/Python/repl.py
index 93b5e23..0a9ee14 100644
--- a/src/Dyna/Backend/Python/repl.py
+++ b/src/Dyna/Backend/Python/repl.py
@@ -279,6 +279,8 @@ class REPL(cmd.Cmd, object):
         print 'Changes'
         print '======='
         for x, v in sorted(changed.items()):
+            if x.fn.startswith('$rule/'):
+                continue
             print '%s = %s.' % (x, _repr(v))
         print
 
diff --git a/src/Dyna/Backend/Python/stdlib.py b/src/Dyna/Backend/Python/stdlib.py
index 0b62668..336a5a5 100644
--- a/src/Dyna/Backend/Python/stdlib.py
+++ b/src/Dyna/Backend/Python/stdlib.py
@@ -27,31 +27,28 @@ def pycall(name, *args):
     x = eval(name)(*args)
     return todyna(x)
 
-def todyna(x):
-    if isinstance(x, (list, tuple, set, Counter)):
-        return todynalist(x)
-    return x
-
 def topython(x):
     if isinstance(x, Cons) or x is Nil:
         return x.aslist
     return x
 
-def todynalist(x):
+
+def todynalist(x):    # TODO: get rid of this.
+    return todyna(x)
+
+def todyna(x):
     if isinstance(x, (set, Counter)):
         x = list(x)
         x.sort()
-        return todynalist(x)
-    return _todynalist(list(x))
+        return todyna(x)
+    elif isinstance(x, (list, tuple)):
+        c = Nil
+        for y in reversed(x):
+            c = Cons(todyna(y), c)
+        return c
+    else:
+        return x
 
-def _todynalist(x):
-#    if not x:
-#        return Nil
-#    return Cons(x[0], _todynalist(x[1:]))
-    c = Nil
-    for y in reversed(x):
-        c = Cons(y, c)
-    return c
 
 def get(x, i):
     return x[i]
diff --git a/src/Dyna/Backend/Python/utils.py b/src/Dyna/Backend/Python/utils.py
index b1299b3..1cef4f7 100644
--- a/src/Dyna/Backend/Python/utils.py
+++ b/src/Dyna/Backend/Python/utils.py
@@ -24,6 +24,14 @@ def drepr(vs):
     return '{%s}' %  ', '.join('%s=%s' % (k, _repr(v)) for k,v in vs.iteritems())
 
 
+def user_vars(variables):
+    "Post process the variables past to emit (which passes them to aggregator)."
+    # remove the 'u' prefix on user variables 'uX'
+    # Note: We also ignore user variables with an underscore prefix
+    return tuple((name[1:], val) for name, val in variables
+                 if name.startswith('u') and not name.startswith('u_'))
+
+
 # interactive IPython shell
 ip = InteractiveShellEmbed(banner1 = 'Dropping into IPython\n')
 
diff --git a/test/repl/retract-rule.doctest b/test/repl/retract-rule.doctest
index 5e3313c..039fcd5 100644
--- a/test/repl/retract-rule.doctest
+++ b/test/repl/retract-rule.doctest
@@ -1,32 +1,50 @@
-:- a += 1.
-=============
-a := 1
+> a += 1.
 
-:- b += 1.
-=============
-b := 1
+Changes
+=======
+a = 1.
 
-:- a += 1.
-=============
-a := 2
+> b += 1.
 
-:- rules
+Changes
+=======
+b = 1.
 
-0: a += 1.
+> a += 1.
+
+Changes
+=======
+a = 2.
+
+> rules
+
+Rules
+=====
+  0: a += 1.
   1: b += 1.
   2: a += 1.
 
-:- sol
+> sol
 
 Solution
 ========
-a := 2
-b := 1
+a = 2.
+b = 1.
+
+> retract_rule 0
+
+Changes
+=======
+a = 1.
+
+> retract_rule 1
+
+Changes
+=======
+b = null.
 
-:- retract_rule 0
-:- retract_rule 1
-:- sol
+> sol
 
 Solution
 ========
-a := 1
+a = 1.