diff --git a/app/context/physical_quantity/comparison.py b/app/context/physical_quantity/comparison.py index cb8664c..74c4139 100644 --- a/app/context/physical_quantity/comparison.py +++ b/app/context/physical_quantity/comparison.py @@ -1,25 +1,53 @@ from __future__ import annotations -from collections.abc import Callable +from collections.abc import Callable, Mapping +from dataclasses import dataclass from sympy import Basic, Symbol from lambdafeedback.expression_parsing import parse_expression from lambdafeedback.slr_parsing import ExprNode +from lambdafeedback.units import parse_quantity as parse_physical_quantity from app.context.operators import COMPARISON_OPERATORS +from app.context.physical_quantity.parsing import ParsedQuantity, parsed_quantity from app.evaluation_parameters import EvaluationParameters from app.types import Substitutions NONE_PLACEHOLDER = Symbol('NONE_PLACEHOLDER') -def parse_criterion_sides(criterion: ExprNode, parameters: EvaluationParameters) -> tuple[str, str, Basic, Basic]: - """The left- and right-hand sides of `criterion` as strings, and parsed without simplification.""" +@dataclass(frozen=True) +class CriterionSides: + """The left- and right-hand sides of a criterion, and the quantities that the sides are written in terms of.""" + lhs_string: str + rhs_string: str + lhs: Basic + rhs: Basic + # The response, the answer and any quantity written out in the criterion (e.g. `2 m`), by the + # symbol that stands for them in `lhs` and `rhs` + quantities: Mapping[str, ParsedQuantity] + + +def parse_criterion_sides(criterion: ExprNode, parameters: EvaluationParameters) -> CriterionSides: + """The sides of `criterion`, where a side written out as a quantity is parsed with its units.""" parsing_params = parameters.parsing_parameters["sympy"].replace(simplify=False) - lhs_string = criterion.children[0].content_string() - rhs_string = criterion.children[1].content_string() - return lhs_string, rhs_string, parse_expression(lhs_string, parsing_params), parse_expression(rhs_string, parsing_params) + quantities = dict(parameters.reserved_expressions) + sides = [] + for (name, node) in zip(("criterion_lhs", "criterion_rhs"), criterion.children): + if node.label == "INPUT": + quantity = parse_physical_quantity(node.content_string(), parameters.parsing_parameters["quantity"], name) + quantities[name] = parsed_quantity(quantity) + sides.append(Symbol(name)) + else: + sides.append(parse_expression(node.content_string(), parsing_params)) + return CriterionSides( + lhs_string=criterion.children[0].content_string(), + rhs_string=criterion.children[1].content_string(), + lhs=sides[0], + rhs=sides[1], + quantities=quantities, + ) def _substitute(lhs: Basic, rhs: Basic, substitutions: Substitutions) -> tuple[Basic, Basic]: @@ -28,12 +56,16 @@ def _substitute(lhs: Basic, rhs: Basic, substitutions: Substitutions) -> tuple[B return lhs.subs(local_substitutions), rhs.subs(local_substitutions) -def comparison_function(comparison_operator: str) -> Callable[[Basic, Basic, Substitutions], bool]: +def comparison_function(comparison_operator: str) -> Callable[[Basic, Basic, Substitutions], bool | None]: comparison = COMPARISON_OPERATORS[comparison_operator] - def comparison_function_inner(lhs: Basic, rhs: Basic, substitutions: Substitutions) -> bool: + def comparison_function_inner(lhs: Basic, rhs: Basic, substitutions: Substitutions) -> bool | None: expr0, expr1 = _substitute(lhs, rhs, substitutions) - return bool(comparison((expr0-expr1).cancel().simplify().simplify(), 0)) + try: + return bool(comparison((expr0-expr1).cancel().simplify().simplify(), 0)) + except TypeError: + # The difference still contains symbols, so sympy cannot determine the truth value + return None return comparison_function_inner diff --git a/app/context/physical_quantity/graphs/match.py b/app/context/physical_quantity/graphs/match.py index 33c4324..e688389 100644 --- a/app/context/physical_quantity/graphs/match.py +++ b/app/context/physical_quantity/graphs/match.py @@ -10,7 +10,7 @@ from lambdafeedback.slr_parsing import ExprNode from app.context.graph_helpers import attach_terminal_criterion, outcome_tag -from app.context.physical_quantity.comparison import comparison_function, is_proportional, parse_criterion_sides +from app.context.physical_quantity.comparison import CriterionSides, comparison_function, is_proportional, parse_criterion_sides from app.evaluation_parameters import EvaluationParameters from app.feedback.physical_quantity import feedback_string_generators as physical_quantity_feedback_string_generators @@ -49,67 +49,78 @@ def _missing_or_unexpected(parameters: EvaluationParameters, label: str, lhs_str def _within_tolerance(parameters: EvaluationParameters, lhs_string: str, rhs_string: str, rtol: float, atol: float) -> bool: """Whether the standard values of the response and answer are within the numerical tolerances.""" - value_match = False # TODO: better analysis of where `answer` is found in the criteria so that # numerical tolerances can be applied appropriately - if rtol > 0 or atol > 0: - if (lhs_string == 'answer' and rhs_string == 'response') or (lhs_string == 'response' and rhs_string == 'answer'): - ans = parameters.reserved_expressions["answer"].standard_value.simplify() - res = parameters.reserved_expressions["response"].standard_value.simplify() - if (ans is not None and ans.is_constant()) and (res is not None and res.is_constant()): - if rtol > 0 and (ans != 0): - value_match = bool(abs(float((ans-res)/ans)) < rtol) - elif atol > 0 or (ans == 0): - answer_unit_factor = float(parameters.reserved_expressions["answer"].quantity.unit_factor) - value_match = bool(abs(float(ans-res)) < atol*answer_unit_factor) + if rtol == 0 and atol == 0: + return False + + if not ((lhs_string == 'answer' and rhs_string == 'response') or (lhs_string == 'response' and rhs_string == 'answer')): + return False + + ans_value = parameters.reserved_expressions["answer"].standard_value + res_value = parameters.reserved_expressions["response"].standard_value + + # Quantities with several standard values are a set, and quantities without a value are None + if not (isinstance(ans_value, Basic) and isinstance(res_value, Basic)): + return False + + ans = ans_value.simplify() + res = res_value.simplify() + + value_match = False + + if ans.is_constant() and res.is_constant(): + if rtol > 0 and (ans != 0): + value_match = bool(abs(float((ans-res)/ans)) < rtol) + elif atol > 0 or (ans == 0): + answer_unit_factor = float(parameters.reserved_expressions["answer"].quantity.unit_factor) + value_match = bool(abs(float(ans-res)) < atol*answer_unit_factor) return value_match -def _quantity_match( - parameters: EvaluationParameters, label: str, lhs: Basic, rhs: Basic, lhs_string: str, rhs_string: str, rtol: float, atol: float -) -> Evaluate: - reserved_expressions = parameters.reserved_expressions.items() +def _quantity_match(parameters: EvaluationParameters, label: str, sides: CriterionSides, rtol: float, atol: float) -> Evaluate: + quantities = sides.quantities.items() def quantity_match(_response: Any) -> ReachedCriteria: - missing_or_unexpected = _missing_or_unexpected(parameters, label, lhs_string, rhs_string) + missing_or_unexpected = _missing_or_unexpected(parameters, label, sides.lhs_string, sides.rhs_string) if missing_or_unexpected is not None: return missing_or_unexpected - substitutions = [(key, expr.standard_value) for (key, expr) in reserved_expressions] - value_match = is_equal(lhs, rhs, substitutions) + substitutions = [(key, expr.standard_value) for (key, expr) in quantities] + value_match = is_equal(sides.lhs, sides.rhs, substitutions) if value_match is False: - value_match = _within_tolerance(parameters, lhs_string, rhs_string, rtol, atol) + value_match = _within_tolerance(parameters, sides.lhs_string, sides.rhs_string, rtol, atol) - substitutions = [(key, expr.standard_unit) for (key, expr) in reserved_expressions] - unit_match = is_equal(lhs, rhs, substitutions) + substitutions = [(key, expr.standard_unit) for (key, expr) in quantities] + unit_match = is_equal(sides.lhs, sides.rhs, substitutions) - return outcome_tag(label, value_match is True and unit_match is True, {"lhs": lhs_string, "rhs": rhs_string}) + return outcome_tag(label, value_match is True and unit_match is True, {"lhs": sides.lhs_string, "rhs": sides.rhs_string}) return quantity_match -def _dimension_match(parameters: EvaluationParameters, label: str, lhs: Basic, rhs: Basic, lhs_string: str, rhs_string: str) -> Evaluate: - reserved_expressions = parameters.reserved_expressions.items() +def _dimension_match(label: str, sides: CriterionSides) -> Evaluate: + quantities = sides.quantities.items() def dimension_match(_response: Any) -> ReachedCriteria: - substitutions = [(key, expr.dimension) for (key, expr) in reserved_expressions] - dimension_match, _ = is_proportional(lhs, rhs, substitutions) - return outcome_tag(label+"_DIMENSION_MATCH", dimension_match is True, {"lhs": lhs_string, "rhs": rhs_string}) + substitutions = [(key, expr.dimension) for (key, expr) in quantities] + dimension_match, _ = is_proportional(sides.lhs, sides.rhs, substitutions) + return outcome_tag(label+"_DIMENSION_MATCH", dimension_match is True, {"lhs": sides.lhs_string, "rhs": sides.rhs_string}) return dimension_match -def _unit_comparison(parameters: EvaluationParameters, label: str, lhs: Basic, rhs: Basic) -> Evaluate: - reserved_expressions = parameters.reserved_expressions.items() +def _unit_comparison(label: str, sides: CriterionSides) -> Evaluate: + quantities = sides.quantities.items() def unit_comparison(_response: Any) -> ReachedCriteria: - substitutions = [(key, expr.original_unit) for (key, expr) in reserved_expressions] + substitutions = [(key, expr.original_unit) for (key, expr) in quantities] - if is_equal(lhs, rhs, substitutions) is True: + if is_equal(sides.lhs, sides.rhs, substitutions) is True: output_tags = { label+"_UNIT_COMPARISON"+"_IDENTICAL": None } else: - local_substitutions = [(key, expr.quantity.expanded_unit) for (key, expr) in reserved_expressions] - result, ratio = is_proportional(lhs, rhs, local_substitutions) + local_substitutions = [(key, expr.quantity.expanded_unit) for (key, expr) in quantities] + result, ratio = is_proportional(sides.lhs, sides.rhs, local_substitutions) # `is_proportional` returns a ratio whenever the result is True if result is True and ratio is not None and ratio >= 1000: output_tags = { @@ -263,15 +274,15 @@ def criterion_match_node(criterion: ExprNode, parameters: EvaluationParameters, label = criterion.content_string() inputs = criterion.children - lhs_string, rhs_string, lhs, rhs = parse_criterion_sides(criterion, parameters) + sides = parse_criterion_sides(criterion, parameters) _add_match_nodes( graph, END, label, inputs, - quantity_match=_quantity_match(parameters, label, lhs, rhs, lhs_string, rhs_string, rtol, atol), - dimension_match=_dimension_match(parameters, label, lhs, rhs, lhs_string, rhs_string), - unit_comparison=_unit_comparison(parameters, label, lhs, rhs), + quantity_match=_quantity_match(parameters, label, sides, rtol, atol), + dimension_match=_dimension_match(label, sides), + unit_comparison=_unit_comparison(label, sides), ) return graph diff --git a/app/context/physical_quantity/graphs/order.py b/app/context/physical_quantity/graphs/order.py index fbc9df8..1e11415 100644 --- a/app/context/physical_quantity/graphs/order.py +++ b/app/context/physical_quantity/graphs/order.py @@ -14,19 +14,21 @@ def comparison_base_graph( criterion: ExprNode, parameters: EvaluationParameters, comparison_operator: str, label: str | None = None ) -> CriteriaGraph: - reserved_expressions = parameters.reserved_expressions.items() - if label is None: label = criterion.content_string() inputs = criterion.children - _, _, lhs, rhs = parse_criterion_sides(criterion, parameters) + sides = parse_criterion_sides(criterion, parameters) - stardard_forms = [(key, expr.standard_value*expr.standard_unit) for (key, expr) in reserved_expressions] + # A quantity without a unit (standard_unit None) is just its value + stardard_forms = [ + (key, expr.standard_value if expr.standard_unit is None else expr.standard_value*expr.standard_unit) + for (key, expr) in sides.quantities.items() + ] compare = comparison_function(comparison_operator) def compare_evaluate(_response: Any) -> ReachedCriteria: - return outcome_tag(label, compare(lhs, rhs, stardard_forms) is True) + return outcome_tag(label, compare(sides.lhs, sides.rhs, stardard_forms) is True) graph, END = outcome_graph( label, diff --git a/app/context/physical_quantity/parsing.py b/app/context/physical_quantity/parsing.py index 2be6c28..eeb0833 100644 --- a/app/context/physical_quantity/parsing.py +++ b/app/context/physical_quantity/parsing.py @@ -36,6 +36,11 @@ def parse_quantity(name: str, expr: str, parameters: Parameters, evaluation_resu quantity = parse_physical_quantity(expr, parameters["quantity"], name) for index, reverted in enumerate(quantity.reverted_units): evaluation_result.add_feedback(f"{name}_REVERTED_UNIT_{index}", reverted_unit_feedback(reverted)) + return parsed_quantity(quantity) + + +def parsed_quantity(quantity: PhysicalQuantity) -> ParsedQuantity: + """`quantity` with the forms of its value and unit used by the criteria.""" standard_value: Expression | None if quantity.standard_values is None: standard_value = None diff --git a/app/context/symbolic/comparison.py b/app/context/symbolic/comparison.py index 996bf58..34f77f2 100644 --- a/app/context/symbolic/comparison.py +++ b/app/context/symbolic/comparison.py @@ -77,15 +77,10 @@ def create_expressions_for_comparison( def do_comparison_infinite(comparison_symbol: str, lhs_expr: Basic, rhs_expr: Basic) -> bool | None: # When either side is infinite, subtracting (e.g. oo - oo) yields nan, making # the subtraction-based do_comparison unreliable. Compare the sides directly instead. - direct_comparisons = { - "=": lhs_expr == rhs_expr, - ">": lhs_expr > rhs_expr, - ">=": lhs_expr >= rhs_expr, - "<": lhs_expr < rhs_expr, - "<=": lhs_expr <= rhs_expr, - } + # Only the requested comparison is evaluated: ordering complex infinity (zoo) raises TypeError. + comparison = COMPARISON_OPERATORS[comparison_symbol.strip()] try: - return bool(direct_comparisons[comparison_symbol.strip()]) + return bool(comparison(lhs_expr, rhs_expr)) except Exception: return None diff --git a/app/tests/physical_quantity_evaluation_test.py b/app/tests/physical_quantity_evaluation_test.py index d18b0e2..5612d8f 100644 --- a/app/tests/physical_quantity_evaluation_test.py +++ b/app/tests/physical_quantity_evaluation_test.py @@ -548,6 +548,74 @@ def test_physical_quantity_with_rtol_and_imperial_mass_unit(self): result = evaluation_function(res, ans, params, include_test_data=True) assert result["is_correct"] is True + @pytest.mark.parametrize( + "criteria,is_correct", + [ + ("response matches 2 m", False), + ("response matches 2", False), + ("2 m matches response", False), + # Only checks that it no longer raises: the value match ignores `dimension(...)` + ("dimension(response) matches dimension(answer)", None), + ("answer matches answer", True), + ] + ) + def test_matches_criteria_other_than_response_and_answer_when_values_differ(self, criteria, is_correct): + # Used to raise UnboundLocalError in the tolerance check (bug 1 in the bug report) + params = { + 'strict_syntax': False, + 'physical_quantity': True, + 'elementary_functions': True, + 'criteria': criteria, + } + result = evaluation_function("2.1 m", "1.0 m", params) + if is_correct is not None: + assert result["is_correct"] is is_correct + + @pytest.mark.parametrize( + "res,ans,criteria,is_correct", + [ + # Used to raise TypeError: cannot determine truth value of Relational (bug 14 in the bug report) + ("2.1 m", "1 m", "response > 1.5 m", True), + ("2.1 m", "1 m", "response > 1.5 metre", True), + ("2.1 m", "1 m", "response > 3 m", False), + ("2.1 m", "1 m", "response > answer", True), + # Used to raise TypeError when the quantities have no units + ("2.1", "1", "response > answer", True), + ("2.1", "1", "response < 2", False), + ] + ) + def test_order_criteria_against_written_out_quantity(self, res, ans, criteria, is_correct): + params = { + 'strict_syntax': False, + 'physical_quantity': True, + 'elementary_functions': True, + 'criteria': criteria, + } + result = evaluation_function(res, ans, params) + assert result["is_correct"] is is_correct + + @pytest.mark.parametrize( + "res,criteria,is_correct", + [ + # Used to be false for every response (bug 16 in the bug report) + ("2 m", "response matches 2 m", True), + ("2 m", "response matches 2 metre", True), + ("2 m", "2 m matches response", True), + ("2000 mm", "response matches 2 m", True), + ("2.1 m", "response matches 2 m", False), + ("2 s", "response matches 2 m", False), + ] + ) + def test_matches_criteria_against_written_out_quantity(self, res, criteria, is_correct): + params = { + 'strict_syntax': False, + 'physical_quantity': True, + 'elementary_functions': True, + 'criteria': criteria, + } + result = evaluation_function(res, "1 m", params) + assert result["is_correct"] is is_correct + @pytest.mark.parametrize( "ans,res", [ diff --git a/app/tests/symbolic_evaluation_test.py b/app/tests/symbolic_evaluation_test.py index ff1e44f..a87cca1 100644 --- a/app/tests/symbolic_evaluation_test.py +++ b/app/tests/symbolic_evaluation_test.py @@ -1,5 +1,8 @@ import pytest +from sympy import Integer, Symbol, oo, zoo + +from app.context.symbolic.comparison import do_comparison_infinite from app.evaluation import evaluation_function from lambdafeedback.expression_parsing import substitute from lambdafeedback.expression_parsing.symbol_tables import ELEMENTARY_FUNCTIONS as elementary_functions_names @@ -2104,6 +2107,36 @@ def test_infinity_alias(self): result = evaluation_function(response, answer, params) assert result["is_correct"] is False + @pytest.mark.parametrize( + "response,answer,is_correct", + [ + ("1/0", "x", False), + ("zoo", "1", False), + ("oo", "oo", True), + ] + ) + def test_complex_infinity(self, response, answer, is_correct): + # Used to raise TypeError: Invalid comparison of non-real zoo (bug 10 in the bug report) + params = { + 'strict_syntax': False, + 'elementary_functions': True, + } + result = evaluation_function(response, answer, params) + assert result["is_correct"] is is_correct + + @pytest.mark.parametrize( + "comparison_symbol,lhs,rhs,expected", + [ + ("=", zoo, Symbol("x"), False), + ("=", zoo, Integer(1), False), + ("=", oo, oo, True), + (">", zoo, Integer(1), None), + ] + ) + def test_do_comparison_infinite_only_evaluates_requested_comparison(self, comparison_symbol, lhs, rhs, expected): + # The end-to-end test above only reaches this function in some runs, so check it directly + assert do_comparison_infinite(comparison_symbol, lhs, rhs) is expected + def test_equality_expression_mismatch_with_custom_criteria(self): response = "x-0.883x^2=0.251X^3" answer = "0.4842x-0.1163x^3"