Merge pull request #311 from mitodl/sibling_sampling

Rearranging sibling sampling

Author: jolyonb

mitxgraders/formulagrader/formulagrader.py

@@ -8,7 +8,7 @@
 import six
 from voluptuous import Schema, Required, Any, All, Invalid, Length
 from mitxgraders.comparers import equality_comparer
-from mitxgraders.sampling import schema_user_functions_no_random
+from mitxgraders.sampling import schema_user_functions_no_random, DependentSampler
 from mitxgraders.exceptions import MissingInput
 from mitxgraders.baseclasses import ItemGrader
 from mitxgraders.helpers.calc import evaluator, DEFAULT_VARIABLES
@@ -131,7 +131,7 @@ def reset_default_comparer(cls):
         cls.set_default_comparer(equality_comparer)
 
     @staticmethod
-    def eval_and_validate_comparer_params(scoped_eval, comparer_params, siblings_eval):
+    def eval_and_validate_comparer_params(scoped_eval, comparer_params):
         """
         Evaluate the comparer_params, and make sure they contain no references
         to empty siblings.
@@ -149,11 +149,6 @@ def eval_and_validate_comparer_params(scoped_eval, comparer_params, siblings_eva
                    for param in comparer_params]
         # results is a list of (value, EvalMetaData) pairs
         comparer_params_eval = [value for value, _ in results]
-        used_variables = set().union(*[used.variables_used for _, used in results])
-
-        for variable in used_variables:
-            if variable in siblings_eval and np.isnan(siblings_eval[variable]):
-                raise MissingInput('Cannot grade answer, a required input is missing.')
 
         return comparer_params_eval
 
@@ -290,11 +285,13 @@ def gen_evaluations(self, comparer_params, student_input, sibling_formulas,
         comparer_params_evals = []
         student_evals = []
 
-        # Create a list of instructor variables to remove from student evaluation
+        # Create a list of instructor and sibling variables to remove from student evaluation
+        sibling_vars = [key for key in sibling_formulas]
         var_blacklist = []
         for var in self.config['instructor_vars']:
             if var in var_samples[0]:
                 var_blacklist.append(var)
+        var_blacklist += sibling_vars
 
         for i in range(self.config['samples']):
             # Update the functions and variables listings with this sample
@@ -309,33 +306,21 @@ def scoped_eval(expression,
                 return evaluator(expression, variables, functions, suffixes, max_array_dim,
                                  allow_inf=self.config['allow_inf'])
 
-            # Compute the sibling values, and add them to varlist
-            siblings_eval = {
-                key: scoped_eval(sibling_formulas[key])[0]
-                for key in sibling_formulas
-            }
-            varlist.update(siblings_eval)
-
             # Compute expressions
-            comparer_params_eval = self.eval_and_validate_comparer_params(
-                scoped_eval, comparer_params, siblings_eval)
+            comparer_params_eval = self.eval_and_validate_comparer_params(scoped_eval, comparer_params)
             comparer_params_evals.append(comparer_params_eval)
 
             # Before performing student evaluation, scrub the sibling and instructor
             # variables so that students can't use them
-            for key in siblings_eval:
-                del varlist[key]
             for key in var_blacklist:
                 del varlist[key]
 
             student_eval, meta = scoped_eval(student_input)
             student_evals.append(student_eval)
 
-            # TODO: Remove this if statement
             if self.config['debug']:
                 # Put the siblings and instructor variables back in for the debug output
                 varlist.update(var_samples[i])
-                varlist.update(siblings_eval)
                 self.log_eval_info(i, varlist, funclist,
                                    comparer_params_eval=comparer_params_eval,
                                    student_eval=student_eval)
@@ -347,8 +332,15 @@ def raw_check(self, answer, student_input, **kwargs):
 
         # Extract sibling formulas to allow for sampling
         siblings = kwargs.get('siblings', None)
+        # Find sibling variables used in comparer parameters
         comparer_params = answer['expect']['comparer_params']
         required_siblings = self.get_used_vars(comparer_params)
+        # Add in any sibling variables used in DependentSamplers
+        samplers = [self.config['sample_from'][x]
+                    for x in self.config['sample_from']
+                    if isinstance(self.config['sample_from'][x], DependentSampler)]
+        sampler_vars = sum((x.config['depends'] for x in samplers), [])
+        required_siblings = list(set(required_siblings).union(set(sampler_vars)))
         # required_siblings might include some extra variable names, but no matter
         sibling_formulas = self.get_sibling_formulas(siblings, required_siblings)
 

mitxgraders/helpers/math_helpers.py

@@ -16,9 +16,9 @@
 from voluptuous import Schema, Required, Any, All, Length, Coerce
 
 from mitxgraders.baseclasses import ItemGrader
-from mitxgraders.exceptions import InvalidInput, ConfigError
+from mitxgraders.exceptions import InvalidInput, ConfigError, MissingInput
 from mitxgraders.comparers import CorrelatedComparer
-from mitxgraders.sampling import (VariableSamplingSet, RealInterval, DiscreteSet,
+from mitxgraders.sampling import (VariableSamplingSet, RealInterval, DiscreteSet, DependentSampler,
                                   gen_symbols_samples, construct_functions,
                                   construct_constants, construct_suffixes,
                                   schema_user_functions,
@@ -537,6 +537,18 @@ def gen_var_and_func_samples(self, *args):
 
         # Generate the variable list
         variables, sample_from_dict = self.generate_variable_list(expressions)
+
+        # Check if a dictionary of sibling variables has been provided, and sample those too
+        for entry in args:
+            if isinstance(entry, dict):
+                if all([k.startswith('sibling_') for k in entry]):
+                    # This is a sibling dictionary. Add it to the list of variables to sample.
+                    for k in entry:
+                        variables.append(k)
+                        if entry[k] == '':
+                            raise MissingInput('Cannot grade answer, a required input is missing.')
+                        sample_from_dict[k] = DependentSampler(formula=entry[k])
+                    break
 
         # Generate the samples
         var_samples = gen_symbols_samples(variables,
@@ -583,7 +595,7 @@ def generate_variable_list(self, expressions):
                 (full_string, head) = match.groups()
                 variable_list.append(full_string)
                 sample_from_dict[full_string] = sample_from_dict[head]
-        
+
         return variable_list, sample_from_dict
 
     def log_comparison_info(self, comparer, comparer_results):

tests/formulagrader/test_formulagrader.py

@@ -688,6 +688,36 @@ def test_fg_evals_numbered_variables_in_siblings():
         # The second call should provide only the first sibling formula
         assert results[1] == {'sibling_1': 'x_{0}+1'}
 
+def test_siblings_in_dependent_samplers():
+    grader = ListGrader(
+        answers=['1', 'x'],
+        subgraders=[
+            FormulaGrader(),
+            FormulaGrader(variables=['x'], sample_from={'x': DependentSampler(formula='sibling_1+1')})
+        ],
+        ordered=True
+    )
+
+    submission = ['1', '2']
+    expected_result = {
+        'overall_message': '',
+        'input_list': [
+            {'ok': True, 'grade_decimal': 1, 'msg': ''},
+            {'ok': True, 'grade_decimal': 1, 'msg': ''},
+        ]
+    }
+    assert grader(None, submission) == expected_result
+
+    submission = ['2', '3']
+    expected_result = {
+        'overall_message': '',
+        'input_list': [
+            {'ok': False, 'grade_decimal': 0, 'msg': ''},
+            {'ok': True, 'grade_decimal': 1, 'msg': ''},
+        ]
+    }
+    assert grader(None, submission) == expected_result
+
 def test_ng_config():
     """Test that the NumericalGrader config bars unwanted entries"""
     expect = r"not a valid value for dictionary value @ data\[u?'failable_evals'\]. Got 1"

tests/helpers/calc/test_expressions.py

@@ -278,3 +278,6 @@ def test_min_max():
 
 def test_large_numbers():
     assert evaluator('x', variables={'x': 10 ** 30})[0] == 1e30
+
+def test_nan():
+    assert np.isnan(evaluator("x^2", {'x': float('nan')}, {}, {})[0])

tests/test_listgrader.py

@@ -588,6 +588,7 @@ def test_siblings_passed_to_subgrader_check_if_ordered_and_subgrader_list():
                 for _, kwargs in check2.call_args_list:
                     assert kwargs['siblings'] == siblings
 
+
 def test_grouping_unordered_different_lengths():
     """Test that an error is raised if unordered groupings use different numbers of inputs"""
     msg = "Groups must all be the same length when unordered"