Add support for references in formulas for Stock's initial value.

Previously, you couldn't include references in initial formulas,
because we didn't have support for detecting cycles in those references.
Now we do! If there aren't any circular references, then we'll
properly create the initial values, and otherwise we'll throw a
systems.errors.CircularReferences exception.

README.md

@@ -86,6 +86,19 @@ Visualize a model into `dot` for Graphviz:
 
     cat examples/hiring.txt | systems-viz | dot
 
+## Error messages
+
+The parser will do its best to give you a useful error message.
+For example, if you're missing delimiters:
+
+    cat examples/no_delim.txt | systems-run
+    line 1 is missing delimiter '>': "[a] < b @ 25"
+
+At worst, it will give you the line number and line that is
+creating an issue:
+
+    cat examples/invalid_flow.txt | systems-run
+    line 1 could not be parsed: "a > b @ 0..2"
 
 ## Uploading distribution
 

TODO.md

@@ -1,11 +1,6 @@
 
 Some stuff to consider doing:
 
-* need to build a dependency graph across variables, to figure out
-    whether a given system is deterministic and reject if it isn't.
-    the current handling of complex initial values is quite jank,
-    maybe jank enough that we should explicitly reject them until
-    this work is done
 * exporting a model to Excel formula    
 * formulas should support parentheses and do proper operation sequencing
 * support a whitelist of functions being called in formulas, e.g. max, min, etc

docs/spec.md

@@ -39,11 +39,11 @@ and instead you'd use the special syntax for infinite flows:
 
 This `InfiniteFlow` would have initial and maximum values of infinity.
 
-Without going too far into the details, maximum values can be specified using any
-legal formula, more on formulas below, but currently initial values must be either a number:
+Without going too far into the details, initial and maximums can be specified using any
+legal formula, more on formulas below:
 
     Managers(2)
-    Engineers(8, Managers * 2)
+    Engineers(Managers * 4, Managers * 8)
 
 In many cases, though, you'll end up specifying your stocks inline in your
 flows, as opposed to doing them on their own lines, but the syntax
@@ -126,32 +126,16 @@ a `leak`, then the value would be `8`.
 
 ## Formulas
 
-Any flow value and maximum value can be a formula (initial values currently
-reject references because we need to do some work to support detecting circular
-dependencies in formula references, but that is definitely a solvable problem):
+Any flow value, initial value and maximum value can be a formula:
 
     Recruiters(3)
-    Engineers(0, Managers * 8)
+    Engineers(Managers * 4, Managers * 8)
     [Candidates] > Engineers @ Recruiters * 6
     [Candidates] > Managers  @ Recruiters * 3
 
-The above system shows that `Engineers` has a maximum value of
-`Managers * 8` and then shows that both `Engineers` and `Managers`
+The above system shows that `Engineers` has an initial value of `Managers * 4`,
+a maximum value of `Managers * 8` and then shows that both `Engineers` and `Managers`
 grow at multiples of the value of the `Recruiters` stock.
 
 This is also a good example of using the `Recruiters` stock as
 a variable, as it doesn't' actually change over time.
-
-## Error messages
-
-The parser will do its best to give you a useful error message.
-For example, if you're missing delimiters:
-
-    cat examples/no_delim.txt | systems-run
-    line 1 is missing delimiter '>': "[a] < b @ 25"
-
-At worst, it will give you the line number and line that is
-creating an issue:
-
-    cat examples/invalid_flow.txt | systems-run
-    line 1 could not be parsed: "a > b @ 0..2"

setup.py

@@ -5,7 +5,7 @@
 
         setuptools.setup(
                 name="systems",
-                version="0.0.6",
+                version="0.0.7",
                 author="Will Larson",
                 author_email="lethain@gmail.com",
                 description="Describe and run systems diagrams.",

systems/algos.py

@@ -0,0 +1,22 @@
+"Collection of reused algorithms."
+
+
+def find_cycles(in_graph, out_graph):
+    cycles = out_graph.copy()
+    deps = []
+    changed = True
+    while changed:
+        changed = False
+        for node, edges in cycles.items():
+            # if you have no incoming edges and do have outgoing edges,
+            # then trim those edges away
+            if in_graph[node] == [] and edges != []:
+                for edge in edges:
+                    in_graph[edge].remove(node)
+                    cycles[node] = []
+                deps.append(node)
+                changed = True
+
+    has_cycle = any([len(x) > 0 for x in cycles.values()])
+    deps = list(reversed(deps))
+    return has_cycle, cycles, deps

systems/errors.py

@@ -52,6 +52,15 @@ class ReferencesInInitialFormula(FormulaError):
     pass
 
 
+class CircularReferences(IllegalSystemException):
+    def __init__(self, cycle, graph):
+        self.cycle = cycle
+        self.graph = graph
+
+    def __str__(self):
+        return "found cycle '%s' in references '%s'" % (self.cycle, self.graph)
+
+
 class InvalidFormula(IllegalSystemException):
     def __init__(self, formula, msg):
         self.formula = formula

systems/models.py

@@ -10,6 +10,7 @@
 
 from systems.errors import IllegalSourceStock, InvalidFormula
 import systems.lexer
+import systems.algos
 
 DEFAULT_MAXIMUM = float("+inf")
 
@@ -194,13 +195,10 @@ def __init__(self, model):
         self.model = model
         self.state = {}
         for stock in self.model.stocks:
-            refs = stock.initial.references()
-            # TODO: add support for references in initial formula,
-            # but for now let's hard reject to avoid it faily in
-            # unexpected ways
-            if len(refs) > 0:
-                raise systems.errors.ReferencesInInitialFormula(stock.initial)
-            self.state[stock.name] = stock.initial.compute()
+            if stock.name not in self.model.initial_path:
+                self.state[stock.name] = stock.initial.compute(self.state)
+        for name in self.model.initial_path:
+            self.state[name] = self.model.get_stock(name).initial.compute(self.state)
 
     def advance(self):
         deferred = []
@@ -231,6 +229,10 @@ def __init__(self, name):
         self.name = name
         self.stocks = []
         self.flows = []
+        # initial_path is updated in self.validate_initial_cycles(),
+        # and is used to identify the sequence to build up the initial
+        # state file
+        self.initial_path = []
 
     def get_stock(self, name):
         for stock in self.stocks:
@@ -253,17 +255,44 @@ def flow(self, *args, **kwargs):
         return f
 
     def validate(self):
-        for stock in self.stocks:
-            self.validate_formula(stock.initial)
-            self.validate_formula(stock.maximum)
+        self.validate_existing_stocks()
+        self.validate_initial_cycles()
 
-        for flow in self.flows:
-            self.validate_formula(flow.rate.formula)
+    def validate_initial_cycles(self):
+        "References in initial values must not have cycles."
+        inward_refs = {}
+        outward_refs = {}
+        for stock in self.stocks:
+            inward_refs[stock.name] = []
+            outward_refs[stock.name] = []
 
-    def validate_formula(self, formula):
-        refs = formula.references()
+        for stock in self.stocks:
+            refs = stock.initial.references()
+            for ref in refs:
+                outward_refs[stock.name].append(ref)
+                inward_refs[ref].append(stock.name)
+
+        has_cycle, cycles, initial_path = systems.algos.find_cycles(inward_refs, outward_refs)
+        if has_cycle:
+            raise systems.errors.CircularReferences(cycles, outward_refs)
+        self.initial_path = initial_path
+
+    def validate_existing_stocks(self):
+        """
+        All references should point to existing stocks.
+
+        We could implicitly create stocks when they're referenced,
+        but usually these are typos, so I think the least surprising
+        behavior here is to error as opposed to implicitly create.
+        """
         stocks = [s.name for s in self.stocks]
-        for ref in refs:
+        refs = []
+        for stock in self.stocks:
+            refs += [(stock.maximum, x) for x in stock.maximum.references()]
+            refs += [(stock.initial, x) for x in stock.initial.references()]
+        for flow in self.flows:
+            refs += [(flow.rate.formula, x) for x in flow.rate.formula.references()]
+        for formula, ref in refs:
             if ref not in stocks:
                 raise InvalidFormula(formula, "reference to non-existant stock '%s'" % ref)
 

tests/test_parse.py

@@ -73,15 +73,31 @@ def test_parse_invalid_int_or_float(self):
 
     def test_circular_formula_dependency(self):
         txt = """
-        A(100)
+        A(D)
         B(A)
         C(B*3)
         D(C*B)
         """
-        with self.assertRaises(systems.errors.ReferencesInInitialFormula):
+        with self.assertRaises(systems.errors.CircularReferences):
             m = parse.parse(txt, tracebacks=False)
             m.run()
 
+    def test_non_circular_formula_dependency(self):
+        txt = """
+        A(100)
+        B(A)
+        C(B*3)
+        D(C*B)
+        """
+        m = parse.parse(txt)
+        results = m.run()
+        final = results[0]
+        self.assertEqual(100, final['A'])
+        self.assertEqual(100, final['B'])
+        self.assertEqual(300, final['C'])
+        self.assertEqual(30000, final['D'])
+
+
     def test_conflicting_stock_values(self):
         txt = """
         a(10) > b @ 1