From 64256d50610aa1d10fa3af12d96f9a8f1ffc545b Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 9 Aug 2018 09:30:59 -0400 Subject: [PATCH 1/4] make MathArrayError inherit from CalcError --- mitxgraders/helpers/calc/exceptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mitxgraders/helpers/calc/exceptions.py b/mitxgraders/helpers/calc/exceptions.py index b2f6657a..b6b85508 100644 --- a/mitxgraders/helpers/calc/exceptions.py +++ b/mitxgraders/helpers/calc/exceptions.py @@ -54,7 +54,7 @@ class ArgumentError(DomainError): """ pass -class MathArrayError(StudentFacingError): +class MathArrayError(CalcError): """ Thrown by MathArray when anticipated errors are made. """ From cc933c4cd6ba2b36c525800c6cf2f3d6d0c38fb1 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 9 Aug 2018 09:32:50 -0400 Subject: [PATCH 2/4] convert np numbers to builtins during handle_node --- mitxgraders/helpers/calc/calc.py | 34 +++++++++++++++++++++-- tests/formulagrader/test_formulagrader.py | 2 +- tests/helpers/calc/test_calc_arrays.py | 25 +++++++++++++++++ 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/mitxgraders/helpers/calc/calc.py b/mitxgraders/helpers/calc/calc.py index 0a010409..3c4a567b 100644 --- a/mitxgraders/helpers/calc/calc.py +++ b/mitxgraders/helpers/calc/calc.py @@ -342,6 +342,36 @@ def evaluator(formula, suffixes=math_interpreter.suffixes_used) return result, usage +def cast_np_numeric_as_builtin(obj): + """ + Cast numpy numeric types as builtin python types. + + NOTE: We do this because instances of np.number have their own __radd__ + method for handling arrays, which circumvents MathArray's type/shape + checking. + + Examples: + >>> import numpy as np + >>> x = 1.0 + >>> x64 = np.float64(x) + >>> y = 5 + >>> y64 = np.int64(y) + >>> z = 3 + 2j + >>> z128 = np.complex128(z) + >>> examples = [x, x64, y, y64, z, z128] + >>> [type(cast_np_numeric_as_builtin(example)) for example in examples] + [, , , , , ] + + Leaves MathArrays alone: + >>> from mitxgraders.helpers.calc.math_array import MathArray + >>> A = MathArray([1, 2, 3]) + >>> cast_np_numeric_as_builtin(A) + MathArray([1, 2, 3]) + """ + if isinstance(obj, np.number): + return np.asscalar(obj) + return obj + class FormulaParser(object): """ Parses a mathematical expression into a tree that can subsequently be evaluated @@ -564,7 +594,7 @@ def handle_node(node): if not isinstance(node, ParseResults): # Entry is either a (python) number or a string. # Return it directly to the next level up. - return node + return cast_np_numeric_as_builtin(node) node_name = node.getName() if node_name not in self.actions: # pragma: no cover @@ -591,7 +621,7 @@ def handle_node(node): if any(np.any(np.isnan(r)) for r in as_list): return float('nan') - return result + return cast_np_numeric_as_builtin(result) # Find the value of the entire tree # Catch math errors that may arise diff --git a/tests/formulagrader/test_formulagrader.py b/tests/formulagrader/test_formulagrader.py index 1317cf7e..b249bc05 100644 --- a/tests/formulagrader/test_formulagrader.py +++ b/tests/formulagrader/test_formulagrader.py @@ -571,7 +571,7 @@ def test_fg_debug_log(): " 'y': 3.5835764522666245,
\n" " 'z': (1.875174422525385+2.7835460015641598j)}}
\n" "Student Eval: (11.9397106851+2.78354600156j)
\n" - "Compare to: [(11.939710685061661+2.7835460015641598j)]
\n" + "Compare to: [(11.93971068506166+2.7835460015641598j)]
\n" "Comparer Function:
\n" "Comparison Result: {{ 'grade_decimal': 1.0, 'msg': '', 'ok': True}}
\n" "" diff --git a/tests/helpers/calc/test_calc_arrays.py b/tests/helpers/calc/test_calc_arrays.py index a87bff5f..49595dcc 100644 --- a/tests/helpers/calc/test_calc_arrays.py +++ b/tests/helpers/calc/test_calc_arrays.py @@ -76,3 +76,28 @@ def test_triple_vector_product_raises_error(): with raises(CalcError, match=match): evaluator("i*2*i*3*j", variables)[0] + +def test_matharray_errors_make_it_through(): + """ + There is some overlap between this test and the tests in test_math_array. + + Main goal here is to make sure numpy numerics are not introduced during + evaluator(...) calls, because + + np.float64(1.0) + MathArray([1, 2, 3]) + + does not throw an error. + """ + + v = MathArray([1, 2, 3]) + variables = { + 'v': v + } + with raises(CalcError, match="Cannot add/subtract"): + evaluator('v*v + v', variables) + + with raises(CalcError, match="Cannot add/subtract"): + evaluator('v*v - v', variables) + + with raises(CalcError, match="Cannot add/subtract"): + evaluator('(v*v)/v', variables) From 1239810dd2de80c9886e261f89ffa6e1b48177a4 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 9 Aug 2018 09:48:24 -0400 Subject: [PATCH 3/4] improve triple mult vec logic --- mitxgraders/helpers/calc/calc.py | 40 +++++++++++++++++--------- mitxgraders/helpers/calc/math_array.py | 4 +++ tests/helpers/calc/test_calc_arrays.py | 9 ++++-- 3 files changed, 37 insertions(+), 16 deletions(-) diff --git a/mitxgraders/helpers/calc/calc.py b/mitxgraders/helpers/calc/calc.py index 3c4a567b..dd26a29e 100644 --- a/mitxgraders/helpers/calc/calc.py +++ b/mitxgraders/helpers/calc/calc.py @@ -33,7 +33,7 @@ ) from mitxgraders.exceptions import StudentFacingError from mitxgraders.helpers.validatorfuncs import get_number_of_args -from mitxgraders.helpers.calc.math_array import MathArray +from mitxgraders.helpers.calc.math_array import MathArray, is_vector from mitxgraders.helpers.calc.robust_pow import robust_pow from mitxgraders.helpers.calc.mathfuncs import ( DEFAULT_VARIABLES, DEFAULT_FUNCTIONS, DEFAULT_SUFFIXES) @@ -346,9 +346,8 @@ def cast_np_numeric_as_builtin(obj): """ Cast numpy numeric types as builtin python types. - NOTE: We do this because instances of np.number have their own __radd__ - method for handling arrays, which circumvents MathArray's type/shape - checking. + NOTE: We do this because instances of np.number behave badly with MathArray. + See https://github.com/mitodl/mitx-grading-library/issues/124 Examples: >>> import numpy as np @@ -979,6 +978,9 @@ def eval_product(self, parse_result): parse_result: A list of numbers to combine, separated by "*" and "/" [a, "*", b, "/", c] = a*b/c + Has some extra logic to avoid ambiguous vector tirple products. + See https://github.com/mitodl/mitx-grading-library/issues/108 + Usage ===== >>> parser = FormulaParser("1", {"%": 0.01}) @@ -988,23 +990,33 @@ def eval_product(self, parse_result): Traceback (most recent call last): CalcError: Unexpected symbol + in eval_product """ - num_vectors = sum([isinstance(operand, MathArray) and operand.ndim == 1 - for operand in parse_result]) - if num_vectors >= 3: - raise CalcError("Multiplying three or more vectors is ambiguous. " - "Please place parentheses around vector multiplications.") + double_vector_mult_has_occured = False + triple_vector_mult_error = CalcError( + "Multiplying three or more vectors is ambiguous. " + "Please place parentheses around vector multiplications." + ) result = parse_result[0] data = parse_result[1:] while data: op = data.pop(0) - num = data.pop(0) - if op == '*': - result *= num - elif op == '/': - result /= num + value = data.pop(0) + if op == '/': + result /= value + elif op == '*': + if is_vector(value): + if double_vector_mult_has_occured: + raise triple_vector_mult_error + elif is_vector(result): + double_vector_mult_has_occured = True + result *= value else: raise CalcError("Unexpected symbol {} in eval_product".format(op)) + + # Need to cast np numerics as builtins here (in addition to during + # handle_node) because the result is changing shape + result = cast_np_numeric_as_builtin(result) + return result def eval_sum(self, parse_result): diff --git a/mitxgraders/helpers/calc/math_array.py b/mitxgraders/helpers/calc/math_array.py index 59a2bd6a..b22972fe 100644 --- a/mitxgraders/helpers/calc/math_array.py +++ b/mitxgraders/helpers/calc/math_array.py @@ -68,6 +68,10 @@ def is_numberlike_zero_array(obj): def is_square(array): return array.ndim == 2 and array.shape[0] == array.shape[1] +def is_vector(obj): + """Tests if obj is a vector MathArray""" + return isinstance(obj, MathArray) and obj.ndim == 1 + class MathArray(np.ndarray): """ A modification of numpy's ndarray class that behaves more like a mathematician would expect. diff --git a/tests/helpers/calc/test_calc_arrays.py b/tests/helpers/calc/test_calc_arrays.py index 49595dcc..b1a8df86 100644 --- a/tests/helpers/calc/test_calc_arrays.py +++ b/tests/helpers/calc/test_calc_arrays.py @@ -77,6 +77,11 @@ def test_triple_vector_product_raises_error(): with raises(CalcError, match=match): evaluator("i*2*i*3*j", variables)[0] + # Next example should raise an operator shape error, not a triple vec error + match='Cannot divide by a vector' + with raises(CalcError, match=match): + evaluator("i*j/i*i*j", variables)[0] + def test_matharray_errors_make_it_through(): """ There is some overlap between this test and the tests in test_math_array. @@ -99,5 +104,5 @@ def test_matharray_errors_make_it_through(): with raises(CalcError, match="Cannot add/subtract"): evaluator('v*v - v', variables) - with raises(CalcError, match="Cannot add/subtract"): - evaluator('(v*v)/v', variables) + with raises(CalcError, match="Cannot divide"): + evaluator('v*v/v', variables) From 92afb86e3c48305dd0dd72d43aa7b134a30f3884 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 9 Aug 2018 13:02:11 -0400 Subject: [PATCH 4/4] in handle_node, map cast_... over list --- mitxgraders/helpers/calc/calc.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/mitxgraders/helpers/calc/calc.py b/mitxgraders/helpers/calc/calc.py index dd26a29e..0d096ccc 100644 --- a/mitxgraders/helpers/calc/calc.py +++ b/mitxgraders/helpers/calc/calc.py @@ -342,7 +342,7 @@ def evaluator(formula, suffixes=math_interpreter.suffixes_used) return result, usage -def cast_np_numeric_as_builtin(obj): +def cast_np_numeric_as_builtin(obj, map_across_lists=False): """ Cast numpy numeric types as builtin python types. @@ -366,9 +366,19 @@ def cast_np_numeric_as_builtin(obj): >>> A = MathArray([1, 2, 3]) >>> cast_np_numeric_as_builtin(A) MathArray([1, 2, 3]) + + Optionally, map across a list: + >>> target = [np.float64(1.0), np.float64(2.0)] + >>> result = cast_np_numeric_as_builtin(target, map_across_lists=True) + >>> [type(item) for item in result] + [, ] + """ if isinstance(obj, np.number): return np.asscalar(obj) + if map_across_lists and isinstance(obj, list): + return [np.asscalar(item) if isinstance(item, np.number) else item + for item in obj] return obj class FormulaParser(object): @@ -620,7 +630,7 @@ def handle_node(node): if any(np.any(np.isnan(r)) for r in as_list): return float('nan') - return cast_np_numeric_as_builtin(result) + return cast_np_numeric_as_builtin(result, map_across_lists=True) # Find the value of the entire tree # Catch math errors that may arise