Merge pull request #535 from sn6uv/Issue531

Expression caching

mathics/builtin/arithmetic.py

@@ -630,6 +630,9 @@ def apply(self, items, evaluation):
                                 for leaf in leaves[0].leaves]
             number = None
 
+        for leaf in leaves:
+            leaf.last_evaluated = None
+
         if number is not None:
             leaves.insert(0, number)
 

mathics/builtin/control.py

@@ -70,6 +70,10 @@ class CompoundExpression(BinaryOperator):
 
     #> CompoundExpression[]
     #> %
+
+    ## Issue 531
+    #> z = Max[1, 1 + x]; x = 2; z
+     = 3
     """
 
     operator = ';'
@@ -169,6 +173,12 @@ class Switch(Builtin):
     #> a; Switch[b, b]
      : Switch called with 2 arguments. Switch must be called with an odd number of arguments.
      = Switch[b, b]
+
+    ## Issue 531
+    #> z = Switch[b, b];
+     : Switch called with 2 arguments. Switch must be called with an odd number of arguments.
+    #> z
+     = Switch[b, b]
     """
 
     attributes = ('HoldRest',)

mathics/builtin/lists.py

@@ -423,9 +423,9 @@ def process_level(item, assignment):
                                                     assignment.leaves):
                     process_level(sub_item, sub_assignment)
         process_level(result, assign_list)
-        return list_of_list[0]
-    else:
-        return result
+        result = list_of_list[0]
+    result.last_evaluated = None
+    return result
 
 
 def is_in_level(current, depth, start=1, stop=None):
@@ -1418,6 +1418,10 @@ class Cases(Builtin):
      = {2, 9, 10}
     #> Cases[{1, f[2], f[3, 3, 3], 4, f[5, 5]}, f[x__] -> Plus[x]]
      = {2, 3, 3, 3, 5, 5}
+
+    ## Issue 531
+    #> z = f[x, y]; x = 1; Cases[z, _Symbol, Infinity]
+     = {y}
     """
 
 
@@ -1477,6 +1481,10 @@ class DeleteCases(Builtin):
 
     >> DeleteCases[{a, b, 1, c, 2, 3}, _Symbol]
      = {1, 2, 3}
+
+    ## Issue 531
+    #> z = {x, y}; x = 1; DeleteCases[z, _Symbol]
+     = {1}
     """
 
     def apply(self, items, pattern, evaluation):

mathics/builtin/scoping.py

@@ -107,10 +107,6 @@ def apply(self, vars, expr, evaluation):
 
         vars = dict(get_scoping_vars(vars, 'Block', evaluation))
         result = dynamic_scoping(expr.evaluate, vars, evaluation)
-
-        # Variables may have changed: must revalute
-        result.is_evaluated = False
-
         return result
 
 

mathics/core/definitions.py

@@ -49,6 +49,7 @@ def __init__(self, add_builtin=False, builtin_filename=None):
         self.definitions_cache = {}
         self.lookup_cache = {}
         self.proxy = defaultdict(set)
+        self.now = 0    # increments whenever something is updated
 
         if add_builtin:
             from mathics.builtin import modules, contribute
@@ -123,6 +124,28 @@ def clear_definitions_cache(self, name):
         for k in self.proxy.pop(tail, []):
             definitions_cache.pop(k, None)
 
+    def last_changed(self, expr):
+        # timestamp for the most recently changed part of a given expression.
+        if isinstance(expr, Symbol):
+            symb = self.get_definition(expr.get_name(), only_if_exists=True)
+            if symb is None:
+                # symbol doesn't exist so it was never changed
+                return 0
+            try:
+                return symb.changed
+            except AttributeError:
+                # must be system symbol
+                symb.changed = 0
+                return 0
+        result = 0
+        head = expr.get_head()
+        head_changed = self.last_changed(head)
+        result = max(result, head_changed)
+        for leaf in expr.get_leaves():
+            leaf_changed = self.last_changed(leaf)
+            result = max(result, leaf_changed)
+        return result
+
     def get_current_context(self):
         # It's crucial to specify System` in this get_ownvalue() call,
         # otherwise we'll end up back in this function and trigger
@@ -383,40 +406,50 @@ def get_user_definition(self, name, create=True):
             self.clear_cache(name)
             return self.user[name]
 
+    def mark_changed(self, definition):
+        self.now += 1
+        definition.changed = self.now
+
     def reset_user_definition(self, name):
         assert not isinstance(name, Symbol)
         fullname = self.lookup_name(name)
         del self.user[fullname]
         self.clear_cache(fullname)
+        # TODO fix changed
 
     def add_user_definition(self, name, definition):
         assert not isinstance(name, Symbol)
+        self.mark_changed(definition)
         fullname = self.lookup_name(name)
         self.user[fullname] = definition
         self.clear_cache(fullname)
 
     def set_attribute(self, name, attribute):
         definition = self.get_user_definition(self.lookup_name(name))
         definition.attributes.add(attribute)
+        self.mark_changed(definition)
         self.clear_definitions_cache(name)
 
     def set_attributes(self, name, attributes):
         definition = self.get_user_definition(self.lookup_name(name))
         definition.attributes = set(attributes)
+        self.mark_changed(definition)
         self.clear_definitions_cache(name)
 
     def clear_attribute(self, name, attribute):
         definition = self.get_user_definition(self.lookup_name(name))
         if attribute in definition.attributes:
             definition.attributes.remove(attribute)
+        self.mark_changed(definition)
         self.clear_definitions_cache(name)
 
     def add_rule(self, name, rule, position=None):
-        name = self.lookup_name(name)
+        definition = self.get_user_definition(self.lookup_name(name))
         if position is None:
-            result = self.get_user_definition(name).add_rule(rule)
+            result = definition.add_rule(rule)
         else:
-            result = self.get_user_definition(name).add_rule_at(rule, position)
+            result = definition.add_rule_at(rule, position)
+        self.mark_changed(definition)
         self.clear_definitions_cache(name)
         return result
 
@@ -430,27 +463,32 @@ def add_format(self, name, rule, form=''):
             if form not in definition.formatvalues:
                 definition.formatvalues[form] = []
             insert_rule(definition.formatvalues[form], rule)
+        self.mark_changed(definition)
         self.clear_definitions_cache(name)
 
     def add_nvalue(self, name, rule):
         definition = self.get_user_definition(self.lookup_name(name))
         definition.add_rule_at(rule, 'n')
+        self.mark_changed(definition)
         self.clear_definitions_cache(name)
 
     def add_default(self, name, rule):
         definition = self.get_user_definition(self.lookup_name(name))
         definition.add_rule_at(rule, 'default')
+        self.mark_changed(definition)
         self.clear_definitions_cache(name)
 
     def add_message(self, name, rule):
         definition = self.get_user_definition(self.lookup_name(name))
         definition.add_rule_at(rule, 'messages')
+        self.mark_changed(definition)
         self.clear_definitions_cache(name)
 
     def set_values(self, name, values, rules):
         pos = valuesname(values)
         definition = self.get_user_definition(self.lookup_name(name))
         definition.set_values_list(pos, rules)
+        self.mark_changed(definition)
         self.clear_definitions_cache(name)
 
     def get_options(self, name):
@@ -459,6 +497,7 @@ def get_options(self, name):
     def reset_user_definitions(self):
         self.user = {}
         self.clear_cache()
+        # TODO changed
 
     def get_user_definitions(self):
         if six.PY2:
@@ -493,11 +532,13 @@ def set_ownvalue(self, name, value):
     def set_options(self, name, options):
         definition = self.get_user_definition(self.lookup_name(name))
         definition.options = options
+        self.mark_changed(definition)
         self.clear_definitions_cache(name)
 
     def unset(self, name, expr):
         definition = self.get_user_definition(self.lookup_name(name))
         result = definition.remove_rule(expr)
+        self.mark_changed(definition)
         self.clear_definitions_cache(name)
         return result
 

mathics/core/expression.py

@@ -143,6 +143,7 @@ def __new__(cls, *args, **kwargs):
         self.options = None
         self.pattern_sequence = False
         self.unformatted = self
+        self.last_evaluated = None
         return self
 
     def get_attributes(self, definitions):
@@ -510,24 +511,23 @@ def __new__(cls, head, *leaves, **kwargs):
         self.leaves = [from_python(leaf) for leaf in leaves]
 
         self.parse_operator = kwargs.get('parse_operator')
-        self.is_evaluated = False
         return self
 
     def copy(self):
         result = Expression(
             self.head.copy(), *[leaf.copy() for leaf in self.leaves])
         result.options = self.options
         result.original = self
+        # result.last_evaluated = self.last_evaluated
         return result
 
     def shallow_copy(self):
         # this is a minimal, shallow copy: head, leaves are shared with
-        # the original, only the Expression instance is new. we transfer
-        # the is_evaluated state, so we don't reevaluate evaluated stuff.
+        # the original, only the Expression instance is new.
         expr = Expression(self.head)
         expr.leaves = self.leaves
         expr.options = self.options
-        expr.is_evaluated = self.is_evaluated
+        expr.last_evaluated = self.last_evaluated
         return expr
 
     def set_positions(self, position=None):
@@ -777,7 +777,8 @@ def evaluate(self, evaluation):
         if hasattr(self, 'options') and self.options:
             evaluation.options = self.options
         try:
-            if self.is_evaluated:
+            # changed before last evaluated
+            if self.last_evaluated is not None and evaluation.definitions.last_changed(self) <= self.last_evaluated:
                 return self
             head = self.head.evaluate(evaluation)
             attributes = head.get_attributes(evaluation.definitions)
@@ -810,6 +811,7 @@ def eval_range(indices):
                 # rest_range(range(0, 0))
 
             new = Expression(head, *leaves)
+
             if ('System`SequenceHold' not in attributes and    # noqa
                 'System`HoldAllComplete' not in attributes):
                 new = new.flatten(Symbol('Sequence'))
@@ -833,13 +835,16 @@ def flatten_callback(new_leaves, old):
             if 'System`Orderless' in attributes:
                 new.sort()
 
-            new.is_evaluated = True
+            new.last_evaluated = self.last_evaluated
+
             if 'System`Listable' in attributes:
                 done, threaded = new.thread(evaluation)
                 if done:
-                    if not threaded.same(new):
-                        threaded = threaded.evaluate(evaluation)
-                    return threaded
+                    if threaded.same(new):
+                        new.last_evaluated = evaluation.definitions.now
+                        return new
+                    else:
+                        return threaded.evaluate(evaluation)
 
             def rules():
                 rules_names = set()
@@ -862,16 +867,19 @@ def rules():
             for rule in rules():
                 result = rule.apply(new, evaluation, fully=False)
                 if result is not None:
-                    if not result.same(new):
-                        result = result.evaluate(evaluation)
-                    return result
+                    if result.same(new):
+                        new.last_evaluated = evaluation.definitions.now
+                        return new
+                    else:
+                        return result.evaluate(evaluation)
 
             # Expression did not change, re-apply Unevaluated
             for index, leaf in enumerate(new.leaves):
                 if leaf.unevaluated:
                     new.leaves[index] = Expression('Unevaluated', leaf)
 
             new.unformatted = self.unformatted
+            new.last_evaluated = evaluation.definitions.now
             return new
 
         # "Return gets discarded only if it was called from within the r.h.s.

mathics/core/rules.py

@@ -97,9 +97,9 @@ def do_replace(self, vars, options, evaluation):
         # options' values. this is achieved through Expression.evaluate(), which then triggers OptionValue.apply,
         # which in turn consults evaluation.options to return an option value.
 
-        # in order to get there, our expression 'new' (or parts of it) must not have is_evaluated True, since this
+        # in order to get there, our expression 'new' (or parts of it) must have last_evaluated None, since this
         # would make Expression.evaluate() quit early. doing a clean deep copy here, will reset
-        # Expression.is_evaluated for all nodes in the tree.
+        # Expression.last_evaluated for all nodes in the tree.
 
         # if the expression contains OptionValue[] patterns, but options is empty here, we don't need to act, as the
         # expression won't change in that case. the Expression.options would be None anyway, so OptionValue.apply