Commit 477fe670 by Ned Batchelder

All re-randomization has to be bucketed to get a reasonable cache hit rate.

parent 7b26c50e
...@@ -17,9 +17,8 @@ from datetime import datetime ...@@ -17,9 +17,8 @@ from datetime import datetime
import logging import logging
import math import math
import numpy import numpy
import os, os.path import os.path
import re import re
import struct
import sys import sys
from lxml import etree from lxml import etree
...@@ -73,7 +72,7 @@ class LoncapaProblem(object): ...@@ -73,7 +72,7 @@ class LoncapaProblem(object):
- problem_text (string): xml defining the problem - problem_text (string): xml defining the problem
- id (string): identifier for this problem; often a filename (no spaces) - id (string): identifier for this problem; often a filename (no spaces)
- seed (int): random number generator seed (int) - seed (int): random number generator seed (int)
- state (dict): containing the following keys: - state (dict): containing the following keys:
- 'seed' - (int) random number generator seed - 'seed' - (int) random number generator seed
- 'student_answers' - (dict) maps input id to the stored answer for that input - 'student_answers' - (dict) maps input id to the stored answer for that input
...@@ -92,23 +91,20 @@ class LoncapaProblem(object): ...@@ -92,23 +91,20 @@ class LoncapaProblem(object):
if self.system is None: if self.system is None:
raise Exception() raise Exception()
state = state if state else {} state = state or {}
# Set seed according to the following priority: # Set seed according to the following priority:
# 1. Contained in problem's state # 1. Contained in problem's state
# 2. Passed into capa_problem via constructor # 2. Passed into capa_problem via constructor
# 3. Assign from the OS's random number generator
self.seed = state.get('seed', seed) self.seed = state.get('seed', seed)
if self.seed is None: assert self.seed is not None, "Seed must be provided for LoncapaProblem."
self.seed = struct.unpack('i', os.urandom(4))[0]
self.student_answers = state.get('student_answers', {}) self.student_answers = state.get('student_answers', {})
if 'correct_map' in state: if 'correct_map' in state:
self.correct_map.set_dict(state['correct_map']) self.correct_map.set_dict(state['correct_map'])
self.done = state.get('done', False) self.done = state.get('done', False)
self.input_state = state.get('input_state', {}) self.input_state = state.get('input_state', {})
# Convert startouttext and endouttext to proper <text></text> # Convert startouttext and endouttext to proper <text></text>
problem_text = re.sub("startouttext\s*/", "text", problem_text) problem_text = re.sub("startouttext\s*/", "text", problem_text)
problem_text = re.sub("endouttext\s*/", "/text", problem_text) problem_text = re.sub("endouttext\s*/", "/text", problem_text)
......
import fs
import fs.osfs import fs.osfs
import os import os, os.path
from capa.capa_problem import LoncapaProblem
from mock import Mock, MagicMock from mock import Mock, MagicMock
import xml.sax.saxutils as saxutils import xml.sax.saxutils as saxutils
...@@ -36,3 +36,7 @@ test_system = Mock( ...@@ -36,3 +36,7 @@ test_system = Mock(
anonymous_student_id='student', anonymous_student_id='student',
cache=None, cache=None,
) )
def new_loncapa_problem(xml):
"""Construct a `LoncapaProblem` suitable for unit tests."""
return LoncapaProblem(xml, id='1', seed=723, system=test_system)
...@@ -6,9 +6,8 @@ import json ...@@ -6,9 +6,8 @@ import json
import mock import mock
from capa.capa_problem import LoncapaProblem
from .response_xml_factory import StringResponseXMLFactory, CustomResponseXMLFactory from .response_xml_factory import StringResponseXMLFactory, CustomResponseXMLFactory
from . import test_system from . import test_system, new_loncapa_problem
class CapaHtmlRenderTest(unittest.TestCase): class CapaHtmlRenderTest(unittest.TestCase):
...@@ -20,7 +19,7 @@ class CapaHtmlRenderTest(unittest.TestCase): ...@@ -20,7 +19,7 @@ class CapaHtmlRenderTest(unittest.TestCase):
xml_str = "<problem> </problem>" xml_str = "<problem> </problem>"
# Create the problem # Create the problem
problem = LoncapaProblem(xml_str, '1', system=test_system) problem = new_loncapa_problem(xml_str)
# Render the HTML # Render the HTML
rendered_html = etree.XML(problem.get_html()) rendered_html = etree.XML(problem.get_html())
...@@ -39,7 +38,7 @@ class CapaHtmlRenderTest(unittest.TestCase): ...@@ -39,7 +38,7 @@ class CapaHtmlRenderTest(unittest.TestCase):
""") """)
# Create the problem # Create the problem
problem = LoncapaProblem(xml_str, '1', system=test_system) problem = new_loncapa_problem(xml_str)
# Render the HTML # Render the HTML
rendered_html = etree.XML(problem.get_html()) rendered_html = etree.XML(problem.get_html())
...@@ -61,7 +60,7 @@ class CapaHtmlRenderTest(unittest.TestCase): ...@@ -61,7 +60,7 @@ class CapaHtmlRenderTest(unittest.TestCase):
""") """)
# Create the problem # Create the problem
problem = LoncapaProblem(xml_str, '1', system=test_system) problem = new_loncapa_problem(xml_str)
# Render the HTML # Render the HTML
rendered_html = etree.XML(problem.get_html()) rendered_html = etree.XML(problem.get_html())
...@@ -80,7 +79,7 @@ class CapaHtmlRenderTest(unittest.TestCase): ...@@ -80,7 +79,7 @@ class CapaHtmlRenderTest(unittest.TestCase):
""") """)
# Create the problem # Create the problem
problem = LoncapaProblem(xml_str, '1', system=test_system) problem = new_loncapa_problem(xml_str)
# Render the HTML # Render the HTML
rendered_html = etree.XML(problem.get_html()) rendered_html = etree.XML(problem.get_html())
...@@ -98,7 +97,7 @@ class CapaHtmlRenderTest(unittest.TestCase): ...@@ -98,7 +97,7 @@ class CapaHtmlRenderTest(unittest.TestCase):
""") """)
# Create the problem # Create the problem
problem = LoncapaProblem(xml_str, '1', system=test_system) problem = new_loncapa_problem(xml_str)
# Render the HTML # Render the HTML
rendered_html = etree.XML(problem.get_html()) rendered_html = etree.XML(problem.get_html())
...@@ -121,7 +120,7 @@ class CapaHtmlRenderTest(unittest.TestCase): ...@@ -121,7 +120,7 @@ class CapaHtmlRenderTest(unittest.TestCase):
test_system.render_template.return_value = "<div>Input Template Render</div>" test_system.render_template.return_value = "<div>Input Template Render</div>"
# Create the problem and render the HTML # Create the problem and render the HTML
problem = LoncapaProblem(xml_str, '1', system=test_system) problem = new_loncapa_problem(xml_str)
rendered_html = etree.XML(problem.get_html()) rendered_html = etree.XML(problem.get_html())
# Expect problem has been turned into a <div> # Expect problem has been turned into a <div>
...@@ -184,7 +183,7 @@ class CapaHtmlRenderTest(unittest.TestCase): ...@@ -184,7 +183,7 @@ class CapaHtmlRenderTest(unittest.TestCase):
xml_str = CustomResponseXMLFactory().build_xml(**kwargs) xml_str = CustomResponseXMLFactory().build_xml(**kwargs)
# Create the problem and render the html # Create the problem and render the html
problem = LoncapaProblem(xml_str, '1', system=test_system) problem = new_loncapa_problem(xml_str)
# Grade the problem # Grade the problem
correctmap = problem.grade_answers({'1_2_1': 'test'}) correctmap = problem.grade_answers({'1_2_1': 'test'})
...@@ -219,7 +218,7 @@ class CapaHtmlRenderTest(unittest.TestCase): ...@@ -219,7 +218,7 @@ class CapaHtmlRenderTest(unittest.TestCase):
""") """)
# Create the problem and render the HTML # Create the problem and render the HTML
problem = LoncapaProblem(xml_str, '1', system=test_system) problem = new_loncapa_problem(xml_str)
rendered_html = etree.XML(problem.get_html()) rendered_html = etree.XML(problem.get_html())
# Expect that the variable $test has been replaced with its value # Expect that the variable $test has been replaced with its value
......
...@@ -12,9 +12,8 @@ import textwrap ...@@ -12,9 +12,8 @@ import textwrap
import mock import mock
import textwrap import textwrap
from . import test_system from . import new_loncapa_problem
import capa.capa_problem as lcp
from capa.responsetypes import LoncapaProblemError, \ from capa.responsetypes import LoncapaProblemError, \
StudentInputError, ResponseError StudentInputError, ResponseError
from capa.correctmap import CorrectMap from capa.correctmap import CorrectMap
...@@ -33,7 +32,7 @@ class ResponseTest(unittest.TestCase): ...@@ -33,7 +32,7 @@ class ResponseTest(unittest.TestCase):
def build_problem(self, **kwargs): def build_problem(self, **kwargs):
xml = self.xml_factory.build_xml(**kwargs) xml = self.xml_factory.build_xml(**kwargs)
return lcp.LoncapaProblem(xml, '1', system=test_system) return new_loncapa_problem(xml)
def assert_grade(self, problem, submission, expected_correctness, msg=None): def assert_grade(self, problem, submission, expected_correctness, msg=None):
input_dict = {'1_2_1': submission} input_dict = {'1_2_1': submission}
......
...@@ -3,7 +3,9 @@ import datetime ...@@ -3,7 +3,9 @@ import datetime
import hashlib import hashlib
import json import json
import logging import logging
import os
import traceback import traceback
import struct
import sys import sys
from pkg_resources import resource_string from pkg_resources import resource_string
...@@ -23,8 +25,10 @@ from xmodule.util.date_utils import time_to_datetime ...@@ -23,8 +25,10 @@ from xmodule.util.date_utils import time_to_datetime
log = logging.getLogger("mitx.courseware") log = logging.getLogger("mitx.courseware")
# Generated this many different variants of problems with rerandomize=per_student # Generate this many different variants of problems with rerandomize=per_student
NUM_RANDOMIZATION_BINS = 20 NUM_RANDOMIZATION_BINS = 20
# Never produce more than this many different seeds, no matter what.
MAX_RANDOMIZATION_BINS = 1000
def randomization_bin(seed, problem_id): def randomization_bin(seed, problem_id):
...@@ -108,11 +112,7 @@ class CapaModule(CapaFields, XModule): ...@@ -108,11 +112,7 @@ class CapaModule(CapaFields, XModule):
self.close_date = due_date self.close_date = due_date
if self.seed is None: if self.seed is None:
if self.rerandomize == 'never': self.choose_new_seed()
self.seed = 1
elif self.rerandomize == "per_student" and hasattr(self.system, 'seed'):
# see comment on randomization_bin
self.seed = randomization_bin(system.seed, self.location.url)
# Need the problem location in openendedresponse to send out. Adding # Need the problem location in openendedresponse to send out. Adding
# it to the system here seems like the least clunky way to get it # it to the system here seems like the least clunky way to get it
...@@ -156,6 +156,22 @@ class CapaModule(CapaFields, XModule): ...@@ -156,6 +156,22 @@ class CapaModule(CapaFields, XModule):
self.set_state_from_lcp() self.set_state_from_lcp()
assert self.seed is not None
def choose_new_seed(self):
"""Choose a new seed."""
if self.rerandomize == 'never':
self.seed = 1
elif self.rerandomize == "per_student" and hasattr(self.system, 'seed'):
# see comment on randomization_bin
self.seed = randomization_bin(self.system.seed, self.location.url)
else:
self.seed = struct.unpack('i', os.urandom(4))[0]
# So that sandboxed code execution can be cached, but still have an interesting
# number of possibilities, cap the number of different random seeds.
self.seed %= MAX_RANDOMIZATION_BINS
def new_lcp(self, state, text=None): def new_lcp(self, state, text=None):
if text is None: if text is None:
text = self.data text = self.data
...@@ -164,6 +180,7 @@ class CapaModule(CapaFields, XModule): ...@@ -164,6 +180,7 @@ class CapaModule(CapaFields, XModule):
problem_text=text, problem_text=text,
id=self.location.html_id(), id=self.location.html_id(),
state=state, state=state,
seed=self.seed,
system=self.system, system=self.system,
) )
...@@ -831,14 +848,11 @@ class CapaModule(CapaFields, XModule): ...@@ -831,14 +848,11 @@ class CapaModule(CapaFields, XModule):
'error': "Refresh the page and make an attempt before resetting."} 'error': "Refresh the page and make an attempt before resetting."}
if self.rerandomize in ["always", "onreset"]: if self.rerandomize in ["always", "onreset"]:
# reset random number generator seed (note the self.lcp.get_state() # Reset random number generator seed.
# in next line) self.choose_new_seed()
seed = None
else:
seed = self.lcp.seed
# Generate a new problem with either the previous seed or a new seed # Generate a new problem with either the previous seed or a new seed
self.lcp = self.new_lcp({'seed': seed}) self.lcp = self.new_lcp(None)
# Pull in the new problem seed # Pull in the new problem seed
self.set_state_from_lcp() self.set_state_from_lcp()
......
...@@ -550,6 +550,7 @@ class CapaModuleTest(unittest.TestCase): ...@@ -550,6 +550,7 @@ class CapaModuleTest(unittest.TestCase):
def test_reset_problem(self): def test_reset_problem(self):
module = CapaFactory.create(done=True) module = CapaFactory.create(done=True)
module.new_lcp = Mock(wraps=module.new_lcp) module.new_lcp = Mock(wraps=module.new_lcp)
module.choose_new_seed = Mock(wraps=module.choose_new_seed)
# Stub out HTML rendering # Stub out HTML rendering
with patch('xmodule.capa_module.CapaModule.get_problem_html') as mock_html: with patch('xmodule.capa_module.CapaModule.get_problem_html') as mock_html:
...@@ -567,7 +568,8 @@ class CapaModuleTest(unittest.TestCase): ...@@ -567,7 +568,8 @@ class CapaModuleTest(unittest.TestCase):
self.assertEqual(result['html'], "<div>Test HTML</div>") self.assertEqual(result['html'], "<div>Test HTML</div>")
# Expect that the problem was reset # Expect that the problem was reset
module.new_lcp.assert_called_once_with({'seed': None}) module.new_lcp.assert_called_once_with(None)
module.choose_new_seed.assert_called_once_with()
def test_reset_problem_closed(self): def test_reset_problem_closed(self):
module = CapaFactory.create() module = CapaFactory.create()
...@@ -1033,3 +1035,13 @@ class CapaModuleTest(unittest.TestCase): ...@@ -1033,3 +1035,13 @@ class CapaModuleTest(unittest.TestCase):
self.assertTrue(module.seed is not None) self.assertTrue(module.seed is not None)
msg = 'Could not get a new seed from reset after 5 tries' msg = 'Could not get a new seed from reset after 5 tries'
self.assertTrue(success, msg) self.assertTrue(success, msg)
def test_random_seed_bins(self):
# Assert that we are limiting the number of possible seeds.
# Check the conditions that generate random seeds
for rerandomize in ['always', 'per_student', 'true', 'onreset']:
# Get a bunch of seeds, they should all be in 0-999.
for i in range(200):
module = CapaFactory.create(rerandomize=rerandomize)
assert 0 <= module.seed < 1000
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment