diff --git a/app/Dockerfile b/app/Dockerfile index 7832b6e..8b8ef7b 100644 --- a/app/Dockerfile +++ b/app/Dockerfile @@ -10,6 +10,7 @@ RUN pip3 install -r requirements.txt # Copy the evaluation and testing scripts COPY evaluation.py ./app/ +COPY shape.py ./app/ COPY evaluation_test.py ./app/ # Copy Documentation diff --git a/app/docs/user.md b/app/docs/user.md index 9580963..624d45b 100644 --- a/app/docs/user.md +++ b/app/docs/user.md @@ -3,4 +3,7 @@ This function compares two symbolic expression arrays, with any level of nesting (2D, 3D, irregular shape, ...). Each cell is compared using the [`SymbolicEqual`](https://lambda-feedback.github.io/user-documentation/user_eval_function_docs/symbolicEqual/) evaluation function. ## Inputs -This function support all input parameters that [`SymbolicEqual`](https://lambda-feedback.github.io/user-documentation/user_eval_function_docs/symbolicEqual/) uses. \ No newline at end of file +This function support all input parameters that [`SymbolicEqual`](https://lambda-feedback.github.io/user-documentation/user_eval_function_docs/symbolicEqual/) uses. + +## Size mismatches +If the response does not have the same size or shape as the answer (e.g. too many or too few entries, or a list where a single expression was expected), the response is marked incorrect and the feedback explains the difference, e.g. "Your response has 2 rows and 3 columns, but the expected answer has 3 rows and 2 columns." No cells are graded in this case. diff --git a/app/evaluation.py b/app/evaluation.py index feb5a7f..551b8b2 100644 --- a/app/evaluation.py +++ b/app/evaluation.py @@ -1,9 +1,13 @@ -import numpy as np import re from evaluation_function_utils.client import EvaluationFunctionClient from evaluation_function_utils.errors import EvaluationException +try: + from .shape import shape_feedback +except ImportError: + from shape import shape_feedback + client = EvaluationFunctionClient(env_path=".env") @@ -79,11 +83,19 @@ def evaluation_function(response, answer, params): to output the grading response. """ - if not (isinstance(response, list) and isinstance(answer, list)): + if not isinstance(answer, list): raise EvaluationException( - f"Response, Answer given of type {type(response)}, {type(answer)}: types unsupported" + f"Answer given of type {type(answer)}: type unsupported" ) + # Give feedback on a size/shape mismatch rather than failing during grading + size_feedback = shape_feedback(response, answer) + if size_feedback is not None: + return { + "is_correct": False, + "feedback": size_feedback + } + # Grade all cells recursively (to allow any shape input) detailed_feedback, feedback = recursive_grade(params, response, answer, [], [], []) diff --git a/app/evaluation_test.py b/app/evaluation_test.py index 4c045d5..eab13ae 100644 --- a/app/evaluation_test.py +++ b/app/evaluation_test.py @@ -1,13 +1,26 @@ import unittest +from unittest import mock from evaluation_function_utils.client import MissingCredentials + +def import_evaluation(): + try: + from . import evaluation + except ImportError: + import evaluation + return evaluation + + +# The real client needs AWS credentials on creation. Without them, import +# with a mock client so the tests below (which mock the client) can still run. try: - from .evaluation import evaluation_function -except ImportError: - from evaluation import evaluation_function + evaluation = import_evaluation() except MissingCredentials: - pass + with mock.patch("evaluation_function_utils.client.EvaluationFunctionClient"): + evaluation = import_evaluation() + +evaluation_function = evaluation.evaluation_function class TestEvaluationFunction(unittest.TestCase): @@ -29,8 +42,73 @@ class TestEvaluationFunction(unittest.TestCase): as it should. """ - def test_blank(self): - self.assertTrue(True) + def setUp(self): + patcher = mock.patch.object(evaluation, "client") + self.client = patcher.start() + self.addCleanup(patcher.stop) + + def assert_size_feedback(self, response, answer, feedback): + result = evaluation_function(response, answer, {}) + + self.assertEqual(result.get("is_correct"), False) + self.assertEqual(result.get("feedback"), feedback) + # Size mismatches are reported before any cell is graded + self.client.invoke.assert_not_called() + + def test_vector_too_long(self): + self.assert_size_feedback( + ["a", "b", "c"], ["a", "b"], + "Your response has 3 entries, but the expected answer has 2 entries.") + + def test_vector_too_short(self): + self.assert_size_feedback( + ["a"], ["a", "b"], + "Your response has 1 entry, but the expected answer has 2 entries.") + + def test_matrix_wrong_size(self): + self.assert_size_feedback( + [["a", "b", "c"], ["d", "e", "f"]], [["a", "b"], ["c", "d"], ["e", "f"]], + "Your response has 2 rows and 3 columns, but the expected answer has 3 rows and 2 columns.") + + def test_vector_for_matrix(self): + self.assert_size_feedback( + ["a", "b", "c", "d"], [["a", "b"], ["c", "d"]], + "Your response has 4 entries, but the expected answer has 2 rows and 2 columns.") + + def test_list_for_single_expression(self): + self.assert_size_feedback( + ["a", ["b"]], ["a", "b"], + "Entry (2) should be a single expression, not a list.") + + def test_single_expression_for_list(self): + self.assert_size_feedback( + "a", ["a", "b"], + "Your response should be an array of 2 entries.") + + def test_irregular_mismatch(self): + self.assert_size_feedback( + [["a"], ["b", "c", "d"]], [["a"], ["b", "c"]], + "Entry (2) has 3 entries, but should have 2 entries.") + + def test_same_size_is_graded(self): + self.client.invoke.return_value = {"is_correct": True} + + result = evaluation_function([["a", "b"], ["c", "d"]], [["a", "b"], ["c", "d"]], {}) + + self.assertEqual(result.get("is_correct"), True) + self.assertEqual(self.client.invoke.call_count, 4) + + def test_same_shape_different_values_is_graded(self): + self.client.invoke.return_value = {"is_correct": True} + + result = evaluation_function([["a+b", "c"], ["d", "e"]], [["b+a", "c"], ["d", "e"]], {}) + + self.assertEqual(result.get("is_correct"), True) + self.assertEqual(self.client.invoke.call_count, 4) + + def test_answer_not_list(self): + with self.assertRaises(evaluation.EvaluationException): + evaluation_function(["a"], "a", {}) """ NOTE: This function was converted to use the experimental diff --git a/app/shape.py b/app/shape.py new file mode 100644 index 0000000..90555b8 --- /dev/null +++ b/app/shape.py @@ -0,0 +1,77 @@ +import numpy as np + + +def plural(n, word, word_plural=None): + if n == 1: + return f"{n} {word}" + return f"{n} {word_plural or word + 's'}" + + +def entries(n): + return plural(n, "entry", "entries") + + +def describe_shape(shape): + if len(shape) == 1: + return entries(shape[0]) + if len(shape) == 2: + return f"{plural(shape[0], 'row')} and {plural(shape[1], 'column')}" + return "size " + " x ".join(str(n) for n in shape) + + +def array_shape(array): + """ + Return the shape of a regular (rectangular) array, e.g. (2, 3) for a + 2x3 matrix, or None if the array is ragged. + """ + try: + return np.shape(array) + except ValueError: + return None + + +def entry_lengths(array): + """ + Return the length of each top-level entry, with -1 for an entry that + is a single expression rather than a list. + """ + return np.array([len(e) if isinstance(e, list) else -1 for e in array]) + + +def shape_feedback(response, answer): + """ + Return feedback explaining how the size of the response differs + from the answer, or None if they have the same shape. + """ + if not isinstance(response, list): + return f"Your response should be an array of {entries(len(answer))}." + + response_shape = array_shape(response) + answer_shape = array_shape(answer) + + if response_shape is not None and answer_shape is not None: + if response_shape == answer_shape: + return None + return f"Your response has {describe_shape(response_shape)}, "\ + f"but the expected answer has {describe_shape(answer_shape)}." + + # At least one of the arrays is ragged, so numpy can't give its shape + if len(response) != len(answer): + return f"Your response has {entries(len(response))}, "\ + f"but the expected answer has {entries(len(answer))}." + + mismatches = np.argwhere(entry_lengths(response) != entry_lengths(answer)).ravel() + if len(mismatches) == 0: + # The mismatch is more than one level deep + return "Your response does not have the same shape as the expected answer." + + i = mismatches[0] + response_length = int(entry_lengths(response)[i]) + answer_length = int(entry_lengths(answer)[i]) + where = f"Entry ({i + 1})" + + if answer_length == -1: + return f"{where} should be a single expression, not a list." + if response_length == -1: + return f"{where} should be an array of {entries(answer_length)}." + return f"{where} has {entries(response_length)}, but should have {entries(answer_length)}."