From c5953957b61d07077707b5234b24285923e7427f Mon Sep 17 00:00:00 2001 From: Joshua Cooper Date: Thu, 20 Jan 2022 13:17:34 -0500 Subject: [PATCH 1/9] Raise `SyntaxError` instead of returning `None` in `Expression` class --- gillespy2/solvers/cpp/build/expression.py | 32 +++++++++++++++-------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/gillespy2/solvers/cpp/build/expression.py b/gillespy2/solvers/cpp/build/expression.py index a12dda28f..63eb6f777 100644 --- a/gillespy2/solvers/cpp/build/expression.py +++ b/gillespy2/solvers/cpp/build/expression.py @@ -50,17 +50,17 @@ def __init__(self, self.namespace = dict({}) if namespace is None else namespace self.blacklist = dict({}) if blacklist is None else blacklist self.sanitize = sanitize - self.invalid_names = [] - self.invalid_operators = [] + self.invalid_names = set() + self.invalid_operators = set() def check_blacklist(self, operator): operator = type(operator) if operator in self.blacklist: - self.invalid_operators.append(str(self.blacklist.get(operator))) + self.invalid_operators.add(str(self.blacklist.get(operator))) def visit_Name(self, node: "ast.Name"): if node.id not in self.namespace: - self.invalid_names.append(node.id) + self.invalid_names.add(node.id) elif self.sanitize: node.id = self.namespace.get(node.id) self.generic_visit(node) @@ -68,7 +68,7 @@ def visit_Name(self, node: "ast.Name"): def visit_Call(self, node: "ast.Call"): if node.func.id not in self.namespace: - self.invalid_names.append(node.func.id) + self.invalid_names.add(node.func.id) elif self.sanitize: node.func.id = self.namespace.get(node.func.id) self.generic_visit(node) @@ -199,15 +199,25 @@ def validate(self, statement: "str") -> "ExpressionResults": return ExpressionResults(invalid_names=validator.invalid_names, invalid_operators=validator.invalid_operators) - def __get_expr(self, converter: "ExpressionConverter") -> "Optional[str]": + def __get_expr(self, statement: "str", converter: "ExpressionConverter") -> "Optional[str]": validator = Expression.ValidationVisitor(self.namespace, self.blacklist, self.sanitize) validator.visit(converter.tree) + failures_found = [] + if validator.invalid_operators: - return None + base_msg = "Blacklisted operator" + base_msg = f"{base_msg}s" if len(validator.invalid_operators) > 1 else base_msg + failures_found.append(f"{base_msg}: {','.join(validator.invalid_operators)}") if validator.invalid_names: - return None + base_msg = "Cannot resolve species name" + base_msg = f"{base_msg}s" if len(validator.invalid_names) > 1 else base_msg + failures_found.append(f"{base_msg}: {','.join(validator.invalid_names)}") + + if len(failures_found) > 0: + raise SyntaxError(f"Invalid GillesPy2 expression \"{statement}\"\n" + + "\n".join([f"* {msg}" for msg in failures_found])) return converter.get_str() @@ -220,7 +230,7 @@ def getexpr_python(self, statement: "str") -> "Optional[str]": :returns: Python expression string, if valid. Returns None if validation fails. """ expr = ast.parse(statement) - return self.__get_expr(PythonConverter(expr)) + return self.__get_expr(statement, PythonConverter(expr)) def getexpr_cpp(self, statement: "str") -> "Optional[str]": """ @@ -232,7 +242,7 @@ def getexpr_cpp(self, statement: "str") -> "Optional[str]": """ statement = ExpressionConverter.convert_str(statement) expr = ast.parse(statement) - return self.__get_expr(CppConverter(expr)) + return self.__get_expr(statement, CppConverter(expr)) class ExpressionResults: @@ -241,7 +251,7 @@ class ExpressionResults: Any expression items which indicate an invalid expression are listed on an ExpressionResults instance. Empty lists indicate that the expression is valid. """ - def __init__(self, invalid_names: "list[str]" = None, invalid_operators: "list[str]" = None, is_valid=True): + def __init__(self, invalid_names: "set[str]" = None, invalid_operators: "set[str]" = None, is_valid=True): """ Container struct for returning the results of expression validation. From 23c3e662435af75c7c8856d71d0e5528dc636636 Mon Sep 17 00:00:00 2001 From: Joshua Cooper Date: Thu, 20 Jan 2022 13:26:28 -0500 Subject: [PATCH 2/9] Add configurable list of "known function names" to `Expression` class --- gillespy2/solvers/cpp/build/template_gen.py | 7 +++++++ test/test_c_solvers.py | 11 ++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/gillespy2/solvers/cpp/build/template_gen.py b/gillespy2/solvers/cpp/build/template_gen.py index e7e94e371..ad71c379d 100644 --- a/gillespy2/solvers/cpp/build/template_gen.py +++ b/gillespy2/solvers/cpp/build/template_gen.py @@ -39,6 +39,12 @@ class SanitizedModel: "t": "t", } + # Global functions that aren't present in the `math` package, + # as well as functions in Python that have a different name in C++. + function_map = { + "abs": "abs", + } + def __init__(self, model: Model, variable=False): self.model = model self.variable = variable @@ -68,6 +74,7 @@ def __init__(self, model: Model, variable=False): # All "system" namespace entries should always be first. # Otherwise, user-defined identifiers (like, for example, "gamma") might get overwritten. **{name: name for name in math.__dict__.keys()}, + **self.function_map, **self.species_names, **self.parameter_names, **self.reserved_names, diff --git a/test/test_c_solvers.py b/test/test_c_solvers.py index 236491cb4..c9b0d80b2 100644 --- a/test/test_c_solvers.py +++ b/test/test_c_solvers.py @@ -27,6 +27,7 @@ from gillespy2.solvers.cpp import SSACSolver, ODECSolver, TauLeapingCSolver from gillespy2.solvers.cpp import TauHybridCSolver from gillespy2.solvers.cpp.build.expression import Expression, ExpressionConverter +from gillespy2.solvers.cpp.build.template_gen import SanitizedModel class ExpressionTestCase: @@ -85,6 +86,10 @@ class TestCSolvers(unittest.TestCase): ExpressionTestCase({"x": "x", "y": "y", "z": "z"}, "(x^2/y^2/z^2)/x^2/y^2/z^2**1/x**1/y**1/z", [ [5.1, 0.1, 2.0], [0.1, 5.1, 2.0], [2.0, 0.1, 5.1], [2.0, 5.1, 0.1], ]), + # Known, builtin math expression functions work. + ExpressionTestCase({"x": "x"}, "abs(x)", [ + [100.0], [100], [-100.0], [-100], [0], + ]), ] comparisons = [ # Asserts that single comparison expressions work. @@ -189,7 +194,11 @@ def run(args: "list[str]") -> str: def test_expressions(expressions: "list[ExpressionTestCase]", use_bool=False): for entry in expressions: expression = ExpressionConverter.convert_str(entry.expression) - expr = Expression(namespace=entry.args) + expr = Expression(namespace={ + **SanitizedModel.reserved_names, + **SanitizedModel.function_map, + **entry.args, + }) cpp_expr = expr.getexpr_cpp(expression) with self.subTest(msg="Evaluating converted C expressions", expression=entry.expression, From 84cf4e7d03e0e5f2a9a93009726a01b4572593d7 Mon Sep 17 00:00:00 2001 From: Brian Drawert Date: Mon, 17 Jan 2022 17:07:43 -0800 Subject: [PATCH 3/9] catch errors if events are not setup correctly --- gillespy2/solvers/cpp/tau_hybrid_c_solver.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/gillespy2/solvers/cpp/tau_hybrid_c_solver.py b/gillespy2/solvers/cpp/tau_hybrid_c_solver.py index a18a9b850..f7febcb88 100644 --- a/gillespy2/solvers/cpp/tau_hybrid_c_solver.py +++ b/gillespy2/solvers/cpp/tau_hybrid_c_solver.py @@ -108,6 +108,17 @@ def __create_options(cls, sanitized_model: "SanitizedModel") -> "SanitizedModel" assignments.append(str(assign_id)) event_assignment_list.append(assign_str) assign_id += 1 + # Check for "None"s + for a in assignments: + if a is None: raise Exception(f"a is Nonein event={event}") + if event_id is None: raise Exception(f"event_id is None in event={event}") + if trigger is None: raise Exception(f"trigger is None in event={event}") + if delay is None: raise Exception(f"delay is None in event={event}") + if priority is None: raise Exception(f"priority is None in event={event}") + if use_trigger is None: raise Exception(f"use_trigger is None in event={event}") + if use_persist is None: raise Exception(f"use_persist is None in event={event}") + if initial_value is None: raise Exception(f"initial_value is None in event={event}") + assignments: "str" = " AND ".join(assignments) event_list.append( f"EVENT(" From ac008344e33fa148839b4027e81649a6dadb12a5 Mon Sep 17 00:00:00 2001 From: Brian Drawert Date: Mon, 17 Jan 2022 17:18:00 -0800 Subject: [PATCH 4/9] cleanup --- gillespy2/solvers/cpp/tau_hybrid_c_solver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gillespy2/solvers/cpp/tau_hybrid_c_solver.py b/gillespy2/solvers/cpp/tau_hybrid_c_solver.py index f7febcb88..236b58e9c 100644 --- a/gillespy2/solvers/cpp/tau_hybrid_c_solver.py +++ b/gillespy2/solvers/cpp/tau_hybrid_c_solver.py @@ -110,7 +110,7 @@ def __create_options(cls, sanitized_model: "SanitizedModel") -> "SanitizedModel" assign_id += 1 # Check for "None"s for a in assignments: - if a is None: raise Exception(f"a is Nonein event={event}") + if a is None: raise Exception(f"assignment={a} is None in event={event}") if event_id is None: raise Exception(f"event_id is None in event={event}") if trigger is None: raise Exception(f"trigger is None in event={event}") if delay is None: raise Exception(f"delay is None in event={event}") From 821353cc2b2782b7453582fd2f85910744c6365b Mon Sep 17 00:00:00 2001 From: Brian Drawert Date: Mon, 17 Jan 2022 17:57:31 -0800 Subject: [PATCH 5/9] fixed error message --- gillespy2/core/events.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gillespy2/core/events.py b/gillespy2/core/events.py index 9954a510d..8c385e040 100644 --- a/gillespy2/core/events.py +++ b/gillespy2/core/events.py @@ -70,7 +70,7 @@ def __init__(self, name=None, variable=None, expression=None): 'valid string expression') def __str__(self): - return self.variable.name + ': ' + self.expression + return f"{self.variable}: {self.expression}" class EventTrigger(Jsonify): """ @@ -261,4 +261,4 @@ def add_assignment(self, assignment): else: raise ModelError("Unexpected parameter for add_assignment. Parameter must be EventAssignment or list of " "EventAssignments") - return assignment \ No newline at end of file + return assignment From 28904865cc2f2091fdd34e73ca4bbada16ac64c6 Mon Sep 17 00:00:00 2001 From: Brian Drawert Date: Mon, 17 Jan 2022 18:01:46 -0800 Subject: [PATCH 6/9] improved error message --- gillespy2/solvers/cpp/tau_hybrid_c_solver.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gillespy2/solvers/cpp/tau_hybrid_c_solver.py b/gillespy2/solvers/cpp/tau_hybrid_c_solver.py index 236b58e9c..04d6372d4 100644 --- a/gillespy2/solvers/cpp/tau_hybrid_c_solver.py +++ b/gillespy2/solvers/cpp/tau_hybrid_c_solver.py @@ -93,7 +93,8 @@ def __create_options(cls, sanitized_model: "SanitizedModel") -> "SanitizedModel" elif variable in sanitized_model.model.listOfParameters: variable = sanitized_model.model.listOfParameters.get(variable) else: - raise ValueError(f"Invalid event assignment {assign}: received name {variable} " + raise ValueError(f"Error in event={event} " + f"Invalid event assignment {assign}: received name {variable} " f"Must match the name of a valid Species or Parameter.") if isinstance(variable, gillespy2.Species): From 4458adf20131c7392deb3b78d23a00a0d3016418 Mon Sep 17 00:00:00 2001 From: Brian Drawert Date: Mon, 17 Jan 2022 18:09:34 -0800 Subject: [PATCH 7/9] Raise error if element not found, otherwise return error string treated as Species obj --- gillespy2/core/model.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gillespy2/core/model.py b/gillespy2/core/model.py index 00d05135c..fd04c5c3e 100644 --- a/gillespy2/core/model.py +++ b/gillespy2/core/model.py @@ -841,7 +841,8 @@ def get_element(self, ename): return self.get_assignment_rule(ename) if ename in self.listOfFunctionDefinitions: return self.get_function_definition(ename) - return 'Element not found!' + #return 'Element not found!' + raise ModelError(f"model.get_element(): element={ename} not found") def get_best_solver(self): From 6a99f9e6c20abe541cfaade864f91f2225ae21fa Mon Sep 17 00:00:00 2001 From: Brian Drawert Date: Mon, 17 Jan 2022 23:30:56 -0800 Subject: [PATCH 8/9] Fixing bug --- gillespy2/solvers/utilities/solverutils.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/gillespy2/solvers/utilities/solverutils.py b/gillespy2/solvers/utilities/solverutils.py index f5ea54fbe..668f40e42 100644 --- a/gillespy2/solvers/utilities/solverutils.py +++ b/gillespy2/solvers/utilities/solverutils.py @@ -19,6 +19,7 @@ import ast # for dependency graphing import numpy as np from gillespy2.core import log, Species +from gillespy2.core import ModelError """ NUMPY SOLVER UTILITIES BELOW @@ -143,8 +144,11 @@ def species_parse(model, custom_prop_fun): class SpeciesParser(ast.NodeTransformer): def visit_Name(self, node): - if isinstance(model.get_element(node.id), Species): - parsed_species.append(model.get_element(node.id)) + try: + if isinstance(model.get_element(node.id), Species): + parsed_species.append(model.get_element(node.id)) + except ModelError: + pass expr = custom_prop_fun expr = ast.parse(expr, mode='eval') From 70cd5c0d60c17bf0daa84a1c483c7efa4a46a16c Mon Sep 17 00:00:00 2001 From: Joshua Cooper Date: Wed, 26 Jan 2022 11:30:33 -0500 Subject: [PATCH 9/9] Remove dead code from comment --- gillespy2/core/model.py | 1 - 1 file changed, 1 deletion(-) diff --git a/gillespy2/core/model.py b/gillespy2/core/model.py index fd04c5c3e..0c881d77e 100644 --- a/gillespy2/core/model.py +++ b/gillespy2/core/model.py @@ -841,7 +841,6 @@ def get_element(self, ename): return self.get_assignment_rule(ename) if ename in self.listOfFunctionDefinitions: return self.get_function_definition(ename) - #return 'Element not found!' raise ModelError(f"model.get_element(): element={ename} not found")