Time-limit SymPy simplification so numeric logs of large powers can't hang grading

app/context/numerical.py

@@ -1,6 +1,35 @@
 from sympy import pi, N
 
 
+def replace_pi(expr, value=float(pi)):
+    # REMARK: 'pi' should be a reserved symbol but it is sometimes not treated as one, possibly because of input symbols.
+    # Replacing it with its value fixes the issue but a more robust solution should be found for cases where there
+    # are other reserved symbols.
+    pi_symbol = pi
+    for s in expr.free_symbols:
+        if str(s) == 'pi':
+            pi_symbol = s
+    return expr.subs(pi_symbol, value)
+
+
+def numerically_equal(lhs_expr, rhs_expr):
+    """
+    Whether `lhs_expr` and `rhs_expr` are equal when evaluated to 50 digits: compares numbers
+    without simplifying them, which can take practically forever (see `app.time_budget`).
+
+    Returns None if either contains symbols other than `pi` or can't be evaluated to a number.
+    """
+    lhs, rhs = replace_pi(lhs_expr, pi), replace_pi(rhs_expr, pi)
+    if lhs.free_symbols or rhs.free_symbols:
+        return None
+    try:
+        difference = abs(complex(N(lhs-rhs, 50)))
+        scale = max(1.0, abs(complex(N(lhs, 50))), abs(complex(N(rhs, 50))))
+    except (TypeError, ValueError, OverflowError):
+        return None
+    return bool(difference <= 1e-30*scale)
+
+
 def compare_numerically(lhs_expr, rhs_expr, parameters_dict):
     """
     Compares `lhs_expr` and `rhs_expr` numerically using the `atol` and `rtol` parameters.
@@ -13,15 +42,6 @@ def compare_numerically(lhs_expr, rhs_expr, parameters_dict):
             or float(parameters_dict.get("atol", 0)) > 0):
         return False
 
-    # REMARK: 'pi' should be a reserved symbol but it is sometimes not treated as one, possibly because of input symbols.
-    # The two lines below this comments fixes the issue but a more robust solution should be found for cases where there
-    # are other reserved symbols.
-    def replace_pi(expr):
-        pi_symbol = pi
-        for s in expr.free_symbols:
-            if str(s) == 'pi':
-                pi_symbol = s
-        return expr.subs(pi_symbol, float(pi))
     # NOTE: This code assumes that the left hand side is the response and the right hand side is the answer
     # Separates LHS and RHS, parses and evaluates them
     res = N(replace_pi(lhs_expr))

app/context/symbolic.py

@@ -11,10 +11,11 @@
 )
 
 from ..preview.symbolic import preview_function
-from .numerical import compare_numerically
+from .numerical import compare_numerically, numerically_equal
 from .set_equivalence import add_set_equivalence_nodes, as_sets
 from ..feedback.symbolic import feedback_generators as symbolic_feedback_string_generators
 from ..feedback.syntactical_forms import PATTERNS as syntactical_forms
+from ..time_budget import comparison_budget
 
 from lambdafeedback.criteria import CriteriaGraph, build_criteria_parser
 
@@ -133,23 +134,28 @@ def check_equality(criterion, parameters_dict, local_substitutions=[]):
     elif not isinstance(lhs_expr, Equality) and isinstance(rhs_expr, Equality):
         result = False
     else:
-        try:
-            either_is_infinite = lhs_expr.is_infinite or rhs_expr.is_infinite
-        except (KeyError, TypeError):
-            either_is_infinite = False
-        if either_is_infinite:
-            result = do_comparison_infinite(criterion.content, lhs_expr, rhs_expr)
-        else:
-            result = do_comparison(criterion.content, lhs_expr-rhs_expr)
-        # There are some types of expression, e.g. those containing hyperbolic trigonometric functions, that can behave
-        # unpredictably when simplification is applied. For that reason we check several different combinations of
-        # simplifications here in order to reduce the likelihood of false negatives.
-        if result is False:
-            result = do_comparison(criterion.content, lhs_expr-rhs_expr.simplify())
-        if result is False:
-            result = do_comparison(criterion.content, lhs_expr.simplify()-rhs_expr)
-        if result is False:
-            result = do_comparison(criterion.content, lhs_expr.simplify()-rhs_expr.simplify())
+        result = None
+        with comparison_budget() as budget:
+            try:
+                either_is_infinite = lhs_expr.is_infinite or rhs_expr.is_infinite
+            except (KeyError, TypeError):
+                either_is_infinite = False
+            if either_is_infinite:
+                result = do_comparison_infinite(criterion.content, lhs_expr, rhs_expr)
+            else:
+                result = do_comparison(criterion.content, lhs_expr-rhs_expr)
+            # There are some types of expression, e.g. those containing hyperbolic trigonometric functions, that can behave
+            # unpredictably when simplification is applied. For that reason we check several different combinations of
+            # simplifications here in order to reduce the likelihood of false negatives.
+            if result is False:
+                result = do_comparison(criterion.content, lhs_expr-rhs_expr.simplify())
+            if result is False:
+                result = do_comparison(criterion.content, lhs_expr.simplify()-rhs_expr)
+            if result is False:
+                result = do_comparison(criterion.content, lhs_expr.simplify()-rhs_expr.simplify())
+        if budget.exceeded:
+            # Simplifying took too long (e.g. 10^8*log(0.1) builds 10^(10^8)), so compare numbers by value instead
+            result = numerically_equal(lhs_expr, rhs_expr) if criterion.content.strip() == "=" else None
 
         if result is False:
             result = compare_numerically(lhs_expr, rhs_expr, parameters_dict)
@@ -159,17 +165,20 @@ def check_equality(criterion, parameters_dict, local_substitutions=[]):
 
 def check_order(criterion, parameters_dict, local_substitutions=[]):
     lhs_expr, rhs_expr = create_expressions_for_comparison(criterion, parameters_dict, local_substitutions)
-    result = do_comparison(criterion.content, lhs_expr-rhs_expr)
+    result = None
+    with comparison_budget():
+        result = do_comparison(criterion.content, lhs_expr-rhs_expr)
     return result
 
 
 def check_proportionality(criterion, parameters_dict, local_substitutions=[]):
     lhs_expr, rhs_expr = create_expressions_for_comparison(criterion, parameters_dict, local_substitutions)
     result = None
-    if lhs_expr.cancel().simplify().simplify() != 0:
-        result = (rhs_expr/lhs_expr).cancel().simplify()
-    elif rhs_expr.cancel().simplify().simplify() != 0:
-        result = (lhs_expr/rhs_expr).cancel().simplify()
+    with comparison_budget():
+        if lhs_expr.cancel().simplify().simplify() != 0:
+            result = (rhs_expr/lhs_expr).cancel().simplify()
+        elif rhs_expr.cancel().simplify().simplify() != 0:
+            result = (lhs_expr/rhs_expr).cancel().simplify()
     if result == 0 or result is None:
         result = False
     else:
@@ -275,13 +284,14 @@ def equality_equivalence(unused_input):
 
         # TODO: Remove when criteria for checking proportionality is implemented
         if isinstance(res, Equality) and isinstance(ans, Equality):
-            if (res.args[0]-res.args[1]).simplify() == 0:
-                symbols_in_equality_ratio = (ans.args[0]-ans.args[1]).simplify().free_symbols
-            elif (ans.args[0]-ans.args[1]).simplify() == 0:
-                symbols_in_equality_ratio = (res.args[0]-res.args[1]).simplify().free_symbols
-            else:
-                symbols_in_equality_ratio = ((res.args[0]-res.args[1])/(ans.args[0]-ans.args[1])).simplify().free_symbols
-            result = {str(s) for s in symbols_in_equality_ratio}.issubset(parameters_dict["parsing_parameters"].constants)
+            with comparison_budget():
+                if (res.args[0]-res.args[1]).simplify() == 0:
+                    symbols_in_equality_ratio = (ans.args[0]-ans.args[1]).simplify().free_symbols
+                elif (ans.args[0]-ans.args[1]).simplify() == 0:
+                    symbols_in_equality_ratio = (res.args[0]-res.args[1]).simplify().free_symbols
+                else:
+                    symbols_in_equality_ratio = ((res.args[0]-res.args[1])/(ans.args[0]-ans.args[1])).simplify().free_symbols
+                result = {str(s) for s in symbols_in_equality_ratio}.issubset(parameters_dict["parsing_parameters"].constants)
         if result is True:
             return {
                 label+"_TRUE": None

app/evaluation.py

@@ -13,6 +13,7 @@
 from .feedback.symbolic import feedback_generators as symbolic_feedback_string_generators
 from .feedback.symbolic import internal_feedback
 from .feedback.tags import FeedbackTag, FeedbackTagName
+from .time_budget import comparison_budget, request_budget
 
 from collections.abc import Mapping
 
@@ -310,50 +311,59 @@ def evaluation_function(response, answer, params, include_test_data=False) -> di
         evaluation_result.add_feedback(
             "NOTATION_WARNING_TRIPLE_FACTORIAL", symbolic_comparison_internal_messages("NOTATION_WARNING_TRIPLE_FACTORIAL")(dict()))
 
-    reserved_expressions_success, reserved_expressions = parse_reserved_expressions(reserved_expressions_strings, parameters, evaluation_result)
-    if reserved_expressions_success is False:
-        return evaluation_result.serialise(include_test_data)
-    reserved_expressions_parsed = {**reserved_expressions["learner"], **reserved_expressions["task"]}
+    # Simplifying can take practically forever (see `time_budget`), so the request
+    # ends with a grade rather than running into the Lambda timeout.
+    with request_budget() as budget:
+        reserved_expressions_success, reserved_expressions = parse_reserved_expressions(reserved_expressions_strings, parameters, evaluation_result)
+        if reserved_expressions_success is False:
+            return evaluation_result.serialise(include_test_data)
+        reserved_expressions_parsed = {**reserved_expressions["learner"], **reserved_expressions["task"]}
 
-    try:
-        preview = context["expression_preview"](response, deepcopy(parameters))["preview"]
-    except Exception:
-        evaluation_result.latex = response
-        evaluation_result.simplified = response
-    else:
-        evaluation_result.latex = preview["latex"]
-        parsed_response = reserved_expressions["learner"]["response"]
-        if isinstance(parsed_response, list) or isinstance(parsed_response, set):
-            evaluation_result.simplified = ", ".join([str(ex.simplify()) for ex in parsed_response])
+        try:
+            preview = context["expression_preview"](response, deepcopy(parameters))["preview"]
+        except Exception:
+            evaluation_result.latex = response
+            evaluation_result.simplified = response
         else:
-            try:
-                evaluation_result.simplified = str(parsed_response.simplify())
-            except Exception:
+            evaluation_result.latex = preview["latex"]
+            parsed_response = reserved_expressions["learner"]["response"]
+            with comparison_budget() as simplification:
+                if isinstance(parsed_response, list) or isinstance(parsed_response, set):
+                    evaluation_result.simplified = ", ".join([str(ex.simplify()) for ex in parsed_response])
+                else:
+                    try:
+                        evaluation_result.simplified = str(parsed_response.simplify())
+                    except Exception:
+                        evaluation_result.simplified = response
+            if simplification.exceeded:
                 evaluation_result.simplified = response
 
-    criteria_parser = context["generate_criteria_parser"](reserved_expressions)
-    criteria = create_criteria_dict(criteria_parser, parameters)
+        criteria_parser = context["generate_criteria_parser"](reserved_expressions)
+        criteria = create_criteria_dict(criteria_parser, parameters)
 
-    evaluation_parameters = FrozenValuesDictionary(
-        {
-            "reserved_expressions_strings": reserved_expressions_strings,
-            "reserved_expressions": reserved_expressions_parsed,
-            "criteria": criteria,
-            "disabled_evaluation_nodes": parameters.get("disabled_evaluation_nodes", set()),
-            "parsing_parameters": parsing_parameters,
-            "evaluation_result": evaluation_result,
-            "syntactical_comparison": parameters.get("syntactical_comparison", False),
-            "multiple_answers_criteria": parameters.get("multiple_answers_criteria", "all"),
-            "numerical": parameters.get("numerical", False),
-            "atol": parameters.get("atol", 0),
-            "rtol": parameters.get("rtol", 0),
-            "custom_feedback": parameters.get("custom_feedback", {}),
-        }
-    )
+        evaluation_parameters = FrozenValuesDictionary(
+            {
+                "reserved_expressions_strings": reserved_expressions_strings,
+                "reserved_expressions": reserved_expressions_parsed,
+                "criteria": criteria,
+                "disabled_evaluation_nodes": parameters.get("disabled_evaluation_nodes", set()),
+                "parsing_parameters": parsing_parameters,
+                "evaluation_result": evaluation_result,
+                "syntactical_comparison": parameters.get("syntactical_comparison", False),
+                "multiple_answers_criteria": parameters.get("multiple_answers_criteria", "all"),
+                "numerical": parameters.get("numerical", False),
+                "atol": parameters.get("atol", 0),
+                "rtol": parameters.get("rtol", 0),
+                "custom_feedback": parameters.get("custom_feedback", {}),
+            }
+        )
 
-    # Performs evaluation of response
-    feedback_procedures = parameters["context"]["feedback_procedure_generator"](evaluation_parameters)
-    generate_feedback(criteria, feedback_procedures, evaluation_parameters)
+        # Performs evaluation of response
+        feedback_procedures = parameters["context"]["feedback_procedure_generator"](evaluation_parameters)
+        generate_feedback(criteria, feedback_procedures, evaluation_parameters)
+    if budget.exceeded:
+        evaluation_result.is_correct = False
+        evaluation_result.add_feedback("TIME_LIMIT_EXCEEDED", symbolic_comparison_internal_messages("TIME_LIMIT_EXCEEDED")(dict()))
 
     result = evaluation_result.serialise(include_test_data)
 

app/feedback/symbolic.py

@@ -16,6 +16,7 @@
     "ABSOLUTE_VALUE_NOTATION_AMBIGUITY": f"Notation in {inputs.get('name', '')} might be ambiguous, use `Abs(.)` instead of `|.|`",
     "BRACKET_NOTATION_MISMATCH": f"Brackets in `{inputs.get('x', '')}` do not match. Every `(`, `[` or `{{` must be closed with the corresponding `)`, `]` or `}}` — different bracket types cannot be mixed to close the same group.",
     "NO_RESPONSE": "No response submitted.",
+    "TIME_LIMIT_EXCEEDED": "The response could not be evaluated in time. Try writing it in a simpler form.",
     "MULTIPLE_ANSWER_FAIL_ALL": "At least one answer or response was incorrect.",
     "MULTIPLE_ANSWER_FAIL_RESPONSE": "At least one response was incorrect.",
     "MULTIPLE_ANSWER_FAIL_ANSWER": "At least one answer is missing in the response.",

app/tests/symbolic_evaluation_test.py

@@ -1,5 +1,8 @@
+import time
+
 import pytest
 
+from .. import time_budget
 from ..evaluation import evaluation_function
 from lambdafeedback.expression_parsing import substitute
 from lambdafeedback.expression_parsing.symbol_tables import ELEMENTARY_FUNCTIONS as elementary_functions_names
@@ -2258,3 +2261,46 @@ def test_greek_letters_in_expressions(self, unicode_expr, letter_expr):
         }
         result = evaluation_function(response, answer, params)
         assert result["is_correct"] is True
+
+    # Simplifying a number such as 10^8*log(0.1) builds 10^(10^8) exactly, which
+    # never finishes, so these rely on the time budgets (see `app.time_budget`)
+    large_power_in_log_params = {
+        "elementary_functions": True,
+        "strict_syntax": False,
+        "physical_quantity": False,
+        "rtol": 0,
+        "atol": 0,
+        "symbols": {"pi": {"latex": r"\pi", "aliases": []}},
+    }
+
+    @pytest.fixture
+    def short_comparison_budget(self, monkeypatch):
+        monkeypatch.setattr(time_budget, "COMPARISON_BUDGET", 1)
+
+    @pytest.mark.parametrize("response,answer", [
+        ("log(0.1)*(10^8/ pi)", "(-1/(100*pi*k))*ln(r)+c"),
+        ("900000+10^8* pi* log(100)", "1.47*10**8"),
+    ])
+    def test_large_power_times_numeric_log_is_graded_in_time(self, short_comparison_budget, response, answer):
+        start = time.monotonic()
+        result = evaluation_function(response, answer, self.large_power_in_log_params)
+        assert result["is_correct"] is False
+        assert time.monotonic()-start < 10
+
+    def test_large_power_times_numeric_log_is_compared_numerically(self, short_comparison_budget):
+        response = "log(0.1)*(10^8/ pi)"
+        params = self.large_power_in_log_params
+        assert evaluation_function(response, "-10^8*log(10)/pi", params)["is_correct"] is True
+        assert evaluation_function(response, "-10^8*log(10)/pi+10^-20", params)["is_correct"] is False
+
+    def test_large_power_times_symbolic_log_is_simplified(self):
+        start = time.monotonic()
+        result = evaluation_function("-10^8/( pi) log(r)", "(-1/(100*pi*k))*ln(r)+c", self.large_power_in_log_params)
+        assert result["is_correct"] is False
+        assert time.monotonic()-start < 5
+
+    def test_evaluation_that_runs_out_of_time(self, monkeypatch):
+        monkeypatch.setattr(time_budget, "REQUEST_BUDGET", 1)
+        result = evaluation_function("log(0.1)*(10^8/ pi)", "(-1/(100*pi*k))*ln(r)+c", self.large_power_in_log_params)
+        assert result["is_correct"] is False
+        assert "TIME_LIMIT_EXCEEDED" in result["tags"]