Commit 5ba77275 by Nimisha Asthagiri

Fix problem weights in persisted grades

TNL-5599
parent 6b4972e1
......@@ -10,13 +10,67 @@ import logging
import random
import sys
from collections import namedtuple
log = logging.getLogger("edx.courseware")
# This is a tuple for holding scores, either from problems or sections.
# Section either indicates the name of the problem or the name of the section
Score = namedtuple("Score", "earned possible graded section module_id")
class ScoreBase(object):
"""
Abstract base class for encapsulating fields of values scores.
Field common to all scores include:
display_name (string) - the display name of the module
module_id (UsageKey) - the location of the module
graded (boolean) - whether or not this module is graded
"""
__metaclass__ = abc.ABCMeta
def __init__(self, graded, display_name, module_id):
self.graded = graded
self.display_name = display_name
self.module_id = module_id
def __eq__(self, other):
if type(other) is type(self):
return self.__dict__ == other.__dict__
return False
def __ne__(self, other):
return not self.__eq__(other)
def __repr__(self):
return u"{class_name}({fields})".format(class_name=self.__class__.__name__, fields=self.__dict__)
class ProblemScore(ScoreBase):
"""
Encapsulates the fields of a Problem's score.
In addition to the fields in ScoreBase, also includes:
raw_earned (float) - raw points earned on this problem
raw_possible (float) - raw points possible to earn on this problem
weighted_earned = earned (float) - weighted value of the points earned
weighted_possible = possible (float) - weighted possible points on this problem
weight (float) - weight of this problem
"""
def __init__(self, raw_earned, raw_possible, weighted_earned, weighted_possible, weight, *args, **kwargs):
super(ProblemScore, self).__init__(*args, **kwargs)
self.raw_earned = raw_earned
self.raw_possible = raw_possible
self.earned = weighted_earned
self.possible = weighted_possible
self.weight = weight
class AggregatedScore(ScoreBase):
"""
Encapsulates the fields of a Subsection's score.
In addition to the fields in ScoreBase, also includes:
tw_earned = earned - total aggregated sum of all weighted earned values
tw_possible = possible - total aggregated sum of all weighted possible values
"""
def __init__(self, tw_earned, tw_possible, *args, **kwargs):
super(AggregatedScore, self).__init__(*args, **kwargs)
self.earned = tw_earned
self.possible = tw_possible
def float_sum(iterable):
......@@ -26,13 +80,14 @@ def float_sum(iterable):
return float(sum(iterable))
def aggregate_scores(scores, section_name="summary", location=None):
def aggregate_scores(scores, display_name="summary", location=None):
"""
scores: A list of Score objects
scores: A list of ScoreBase objects
display_name: The display name for the score object
location: The location under which all objects in scores are located
returns: A tuple (all_total, graded_total).
all_total: A Score representing the total score summed over all input scores
graded_total: A Score representing the score summed over all graded input scores
all_total: A ScoreBase representing the total score summed over all input scores
graded_total: A ScoreBase representing the score summed over all graded input scores
"""
total_correct_graded = float_sum(score.earned for score in scores if score.graded)
total_possible_graded = float_sum(score.possible for score in scores if score.graded)
......@@ -41,10 +96,10 @@ def aggregate_scores(scores, section_name="summary", location=None):
total_possible = float_sum(score.possible for score in scores)
#regardless of whether it is graded
all_total = Score(total_correct, total_possible, False, section_name, location)
all_total = AggregatedScore(total_correct, total_possible, False, display_name, location)
#selecting only graded things
graded_total = Score(total_correct_graded, total_possible_graded, True, section_name, location)
graded_total = AggregatedScore(total_correct_graded, total_possible_graded, True, display_name, location)
return all_total, graded_total
......@@ -220,7 +275,7 @@ class SingleSectionGrader(CourseGrader):
found_score = None
if self.type in grade_sheet:
for score in grade_sheet[self.type]:
if score.section == self.name:
if score.display_name == self.name:
found_score = score
break
......@@ -342,7 +397,7 @@ class AssignmentFormatGrader(CourseGrader):
else:
earned = scores[i].earned
possible = scores[i].possible
section_name = scores[i].section
section_name = scores[i].display_name
percentage = earned / possible
summary_format = u"{section_type} {index} - {name} - {percent:.0%} ({earned:.3n}/{possible:.3n})"
......
......@@ -166,8 +166,8 @@ class TestGatedContent(GatingTestCase, MilestonesTestCaseMixin):
"""
course_grade = CourseGradeFactory(user).create(self.course)
for prob in [self.gating_prob1, self.gated_prob2, self.prob3]:
self.assertIn(prob.location, course_grade.locations_to_weighted_scores)
self.assertNotIn(self.orphan.location, course_grade.locations_to_weighted_scores)
self.assertIn(prob.location, course_grade.locations_to_scores)
self.assertNotIn(self.orphan.location, course_grade.locations_to_scores)
self.assertEquals(course_grade.percent, expected_percent)
......
......@@ -27,7 +27,7 @@ BLOCK_RECORD_LIST_VERSION = 1
# Used to serialize information about a block at the time it was used in
# grade calculation.
BlockRecord = namedtuple('BlockRecord', ['locator', 'weight', 'max_score'])
BlockRecord = namedtuple('BlockRecord', ['locator', 'weight', 'raw_possible', 'graded'])
class BlockRecordList(tuple):
......@@ -98,7 +98,8 @@ class BlockRecordList(tuple):
BlockRecord(
locator=UsageKey.from_string(block["locator"]).replace(course_key=course_key),
weight=block["weight"],
max_score=block["max_score"],
raw_possible=block["raw_possible"],
graded=block["graded"],
)
for block in block_dicts
)
......
......@@ -43,15 +43,15 @@ class CourseGrade(object):
return subsections_by_format
@lazy
def locations_to_weighted_scores(self):
def locations_to_scores(self):
"""
Returns a dict of problem scores keyed by their locations.
"""
locations_to_weighted_scores = {}
locations_to_scores = {}
for chapter in self.chapter_grades:
for subsection_grade in chapter['sections']:
locations_to_weighted_scores.update(subsection_grade.locations_to_weighted_scores)
return locations_to_weighted_scores
locations_to_scores.update(subsection_grade.locations_to_scores)
return locations_to_scores
@lazy
def grade_value(self):
......@@ -113,7 +113,7 @@ class CourseGrade(object):
grade_summary['percent'] = self.percent
grade_summary['grade'] = self.letter_grade
grade_summary['totaled_scores'] = self.subsection_grade_totals_by_format
grade_summary['raw_scores'] = list(self.locations_to_weighted_scores.itervalues())
grade_summary['raw_scores'] = list(self.locations_to_scores.itervalues())
return grade_summary
......@@ -141,7 +141,7 @@ class CourseGrade(object):
subsections_total = sum(len(x) for x in self.subsection_grade_totals_by_format.itervalues())
subsections_read = len(subsection_grade_factory._unsaved_subsection_grades) # pylint: disable=protected-access
subsections_created = subsections_total - subsections_read
blocks_total = len(self.locations_to_weighted_scores)
blocks_total = len(self.locations_to_scores)
if not read_only:
subsection_grade_factory.bulk_create_unsaved()
......@@ -166,8 +166,8 @@ class CourseGrade(object):
composite module (a vertical or section ) the scores will be the sums of
all scored problems that are children of the chosen location.
"""
if location in self.locations_to_weighted_scores:
score, _ = self.locations_to_weighted_scores[location]
if location in self.locations_to_scores:
score = self.locations_to_scores[location]
return score.earned, score.possible
children = self.course_structure.get_children(location)
earned = 0.0
......
......@@ -11,12 +11,11 @@ from courseware.model_data import ScoresClient
from lms.djangoapps.grades.scores import get_score, possibly_scored
from lms.djangoapps.grades.models import BlockRecord, PersistentSubsectionGrade
from lms.djangoapps.grades.config.models import PersistentGradesEnabledFlag
from lms.djangoapps.grades.transformer import GradesTransformer
from student.models import anonymous_id_for_user, User
from submissions import api as submissions_api
from traceback import format_exc
from xmodule import block_metadata_utils, graders
from xmodule.graders import Score
from xmodule.graders import AggregatedScore
log = getLogger(__name__)
......@@ -54,62 +53,47 @@ class SubsectionGrade(object):
self.graded_total = None # aggregated grade for all graded problems
self.all_total = None # aggregated grade for all problems, regardless of whether they are graded
self.locations_to_weighted_scores = OrderedDict() # dict of problem locations to (Score, weight) tuples
self._scores = None
self.locations_to_scores = OrderedDict() # dict of problem locations to ProblemScore
@property
def scores(self):
"""
List of all problem scores in the subsection.
"""
if self._scores is None:
self._scores = [score for score, _ in self.locations_to_weighted_scores.itervalues()]
return self._scores
return self.locations_to_scores.values()
def init_from_structure(self, student, course_structure, scores_client, submissions_scores):
def init_from_structure(self, student, course_structure, submissions_scores, csm_scores):
"""
Compute the grade of this subsection for the given student and course.
"""
assert self._scores is None
for descendant_key in course_structure.post_order_traversal(
filter_func=possibly_scored,
start_node=self.location,
):
self._compute_block_score(
student, descendant_key, course_structure, scores_client, submissions_scores, persisted_values={},
)
self._compute_block_score(descendant_key, course_structure, submissions_scores, csm_scores)
self.all_total, self.graded_total = graders.aggregate_scores(self.scores, self.display_name, self.location)
self._log_event(log.info, u"init_from_structure", student)
def init_from_model(self, student, model, course_structure, scores_client, submissions_scores):
def init_from_model(self, student, model, course_structure, submissions_scores, csm_scores):
"""
Load the subsection grade from the persisted model.
"""
assert self._scores is None
for block in model.visible_blocks.blocks:
persisted_values = {'weight': block.weight, 'possible': block.max_score}
self._compute_block_score(
student,
block.locator,
course_structure,
scores_client,
submissions_scores,
persisted_values
)
self._compute_block_score(block.locator, course_structure, submissions_scores, csm_scores, block)
self.graded_total = Score(
earned=model.earned_graded,
possible=model.possible_graded,
self.graded_total = AggregatedScore(
tw_earned=model.earned_graded,
tw_possible=model.possible_graded,
graded=True,
section=self.display_name,
display_name=self.display_name,
module_id=self.location,
)
self.all_total = Score(
earned=model.earned_all,
possible=model.possible_all,
self.all_total = AggregatedScore(
tw_earned=model.earned_all,
tw_possible=model.possible_all,
graded=False,
section=self.display_name,
display_name=self.display_name,
module_id=self.location,
)
self._log_event(log.info, u"init_from_model", student)
......@@ -140,12 +124,11 @@ class SubsectionGrade(object):
def _compute_block_score(
self,
student,
block_key,
course_structure,
scores_client,
submissions_scores,
persisted_values,
csm_scores,
persisted_block=None,
):
"""
Compute score for the given block. If persisted_values
......@@ -154,54 +137,14 @@ class SubsectionGrade(object):
block = course_structure[block_key]
if getattr(block, 'has_score', False):
possible = persisted_values.get('possible', None)
weight = persisted_values.get('weight', getattr(block, 'weight', None))
(earned, possible) = get_score(
student,
block,
scores_client,
problem_score = get_score(
submissions_scores,
weight,
possible,
csm_scores,
persisted_block,
block,
)
if earned is not None or possible is not None:
# There's a chance that the value of graded is not the same
# value when the problem was scored. Since we get the value
# from the block_structure.
#
# Cannot grade a problem with a denominator of 0.
# TODO: None > 0 is not python 3 compatible.
block_graded = self._get_explicit_graded(block, course_structure) if possible > 0 else False
self.locations_to_weighted_scores[block.location] = (
Score(
earned,
possible,
block_graded,
block_metadata_utils.display_name_with_default_escaped(block),
block.location,
),
weight,
)
def _get_explicit_graded(self, block, course_structure):
"""
Returns the explicit graded field value for the given block
"""
field_value = course_structure.get_transformer_block_field(
block.location,
GradesTransformer,
GradesTransformer.EXPLICIT_GRADED_FIELD_NAME
)
# Set to True if grading is not explicitly disabled for
# this block. This allows us to include the block's score
# in the aggregated self.graded_total, regardless of the
# inherited graded value from the subsection. (TNL-5560)
return True if field_value is None else field_value
if problem_score:
self.locations_to_scores[block_key] = problem_score
def _persisted_model_params(self, student):
"""
......@@ -226,9 +169,9 @@ class SubsectionGrade(object):
Returns the list of visible blocks.
"""
return [
BlockRecord(location, weight, score.possible)
for location, (score, weight) in
self.locations_to_weighted_scores.iteritems()
BlockRecord(location, score.weight, score.raw_possible, score.graded)
for location, score in
self.locations_to_scores.iteritems()
]
def _log_event(self, log_func, log_statement, student):
......@@ -283,7 +226,7 @@ class SubsectionGradeFactory(object):
if not subsection_grade:
subsection_grade = SubsectionGrade(subsection, self.course)
subsection_grade.init_from_structure(
self.student, block_structure, self._scores_client, self._submissions_scores
self.student, block_structure, self._submissions_scores, self._csm_scores,
)
if PersistentGradesEnabledFlag.feature_enabled(self.course.id):
if read_only:
......@@ -313,7 +256,7 @@ class SubsectionGradeFactory(object):
block_structure = self._get_block_structure(block_structure)
subsection_grade = SubsectionGrade(subsection, self.course)
subsection_grade.init_from_structure(
self.student, block_structure, self._scores_client, self._submissions_scores
self.student, block_structure, self._submissions_scores, self._csm_scores
)
if PersistentGradesEnabledFlag.feature_enabled(self.course.id):
......@@ -323,7 +266,7 @@ class SubsectionGradeFactory(object):
return subsection_grade
@lazy
def _scores_client(self):
def _csm_scores(self):
"""
Lazily queries and returns all the scores stored in the user
state (in CSM) for the course, while caching the result.
......@@ -351,7 +294,7 @@ class SubsectionGradeFactory(object):
if saved_subsection_grade:
subsection_grade = SubsectionGrade(subsection, self.course)
subsection_grade.init_from_model(
self.student, saved_subsection_grade, block_structure, self._scores_client, self._submissions_scores
self.student, saved_subsection_grade, block_structure, self._submissions_scores, self._csm_scores,
)
return subsection_grade
......
......@@ -2,24 +2,29 @@
Test grade calculation.
"""
import ddt
from django.http import Http404
import itertools
from mock import patch
from nose.plugins.attrib import attr
from opaque_keys.edx.locations import SlashSeparatedCourseKey
from courseware.tests.helpers import (
LoginEnrollmentTestCase,
get_request_for_user
)
from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory
from courseware.tests.helpers import get_request_for_user
from lms.djangoapps.course_blocks.api import get_course_blocks
from student.tests.factories import UserFactory
from student.models import CourseEnrollment
from xmodule.block_metadata_utils import display_name_with_default_escaped
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.graders import ProblemScore
from .utils import answer_problem
from .. import course_grades
from ..course_grades import summary as grades_summary
from ..new.course_grade import CourseGradeFactory
from ..new.subsection_grade import SubsectionGradeFactory
def _grade_with_errors(student, course):
......@@ -36,6 +41,17 @@ def _grade_with_errors(student, course):
return grades_summary(student, course)
def _create_problem_xml():
"""
Creates and returns XML for a multiple choice response problem
"""
return MultipleChoiceResponseXMLFactory().build_xml(
question_text='The correct answer is Choice 3',
choices=[False, False, True, False],
choice_names=['choice_0', 'choice_1', 'choice_2', 'choice_3']
)
@attr(shard=1)
class TestGradeIteration(SharedModuleStoreTestCase):
"""
......@@ -137,6 +153,101 @@ class TestGradeIteration(SharedModuleStoreTestCase):
return students_to_gradesets, students_to_errors
@ddt.ddt
class TestWeightedProblems(SharedModuleStoreTestCase):
"""
Test scores and grades with various problem weight values.
"""
@classmethod
def setUpClass(cls):
super(TestWeightedProblems, cls).setUpClass()
cls.course = CourseFactory.create()
cls.chapter = ItemFactory.create(parent=cls.course, category="chapter", display_name="chapter")
cls.sequential = ItemFactory.create(parent=cls.chapter, category="sequential", display_name="sequential")
cls.vertical = ItemFactory.create(parent=cls.sequential, category="vertical", display_name="vertical1")
problem_xml = _create_problem_xml()
cls.problems = []
for i in range(2):
cls.problems.append(
ItemFactory.create(
parent=cls.vertical,
category="problem",
display_name="problem_{}".format(i),
data=problem_xml,
)
)
def setUp(self):
super(TestWeightedProblems, self).setUp()
self.user = UserFactory()
self.request = get_request_for_user(self.user)
def _verify_grades(self, raw_earned, raw_possible, weight, expected_score):
"""
Verifies the computed grades are as expected.
"""
with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred):
# pylint: disable=no-member
for problem in self.problems:
problem.weight = weight
self.store.update_item(problem, self.user.id)
self.store.publish(self.course.location, self.user.id)
course_structure = get_course_blocks(self.request.user, self.course.location)
# answer all problems
for problem in self.problems:
answer_problem(self.course, self.request, problem, score=raw_earned, max_value=raw_possible)
# get grade
subsection_grade = SubsectionGradeFactory(
self.request.user, self.course, course_structure
).update(self.sequential)
# verify all problem grades
for problem in self.problems:
problem_score = subsection_grade.locations_to_scores[problem.location]
expected_score.display_name = display_name_with_default_escaped(problem)
expected_score.module_id = problem.location
self.assertEquals(problem_score, expected_score)
# verify subsection grades
self.assertEquals(subsection_grade.all_total.earned, expected_score.earned * len(self.problems))
self.assertEquals(subsection_grade.all_total.possible, expected_score.possible * len(self.problems))
@ddt.data(
*itertools.product(
(0.0, 0.5, 1.0, 2.0), # raw_earned
(-2.0, -1.0, 0.0, 0.5, 1.0, 2.0), # raw_possible
(-2.0, -1.0, -0.5, 0.0, 0.5, 1.0, 2.0, 50.0, None), # weight
)
)
@ddt.unpack
def test_problem_weight(self, raw_earned, raw_possible, weight):
use_weight = weight is not None and raw_possible != 0
if use_weight:
expected_w_earned = raw_earned / raw_possible * weight
expected_w_possible = weight
else:
expected_w_earned = raw_earned
expected_w_possible = raw_possible
expected_graded = expected_w_possible > 0
expected_score = ProblemScore(
raw_earned=raw_earned,
raw_possible=raw_possible,
weighted_earned=expected_w_earned,
weighted_possible=expected_w_possible,
weight=weight,
graded=expected_graded,
display_name=None, # problem-specific, filled in by _verify_grades
module_id=None, # problem-specific, filled in by _verify_grades
)
self._verify_grades(raw_earned, raw_possible, weight, expected_score)
class TestScoreForModule(SharedModuleStoreTestCase):
"""
Test the method that calculates the score for a given block based on the
......
......@@ -71,8 +71,8 @@ class GradesModelTestCase(TestCase):
block_type='problem',
block_id='block_id_b'
)
self.record_a = BlockRecord(locator=self.locator_a, weight=1, max_score=10)
self.record_b = BlockRecord(locator=self.locator_b, weight=1, max_score=10)
self.record_a = BlockRecord(locator=self.locator_a, weight=1, raw_possible=10, graded=False)
self.record_b = BlockRecord(locator=self.locator_b, weight=1, raw_possible=10, graded=True)
@ddt.ddt
......@@ -88,29 +88,31 @@ class BlockRecordTest(GradesModelTestCase):
Tests creation of a BlockRecord.
"""
weight = 1
max_score = 10
raw_possible = 10
record = BlockRecord(
self.locator_a,
weight,
max_score,
raw_possible,
graded=False,
)
self.assertEqual(record.locator, self.locator_a)
@ddt.data(
(0, 0, "0123456789abcdef"),
(1, 10, 'totally_a_real_block_key'),
("BlockRecord is", "a dumb data store", "with no validation"),
(0, 0, "0123456789abcdef", True),
(1, 10, 'totally_a_real_block_key', False),
("BlockRecord is", "a dumb data store", "with no validation", None),
)
@ddt.unpack
def test_serialization(self, weight, max_score, block_key):
def test_serialization(self, weight, raw_possible, block_key, graded):
"""
Tests serialization of a BlockRecord using the _asdict() method.
"""
record = BlockRecord(block_key, weight, max_score)
record = BlockRecord(block_key, weight, raw_possible, graded)
expected = OrderedDict([
("locator", block_key),
("weight", weight),
("max_score", max_score),
("raw_possible", raw_possible),
("graded", graded),
])
self.assertEqual(expected, record._asdict())
......@@ -134,7 +136,12 @@ class VisibleBlocksTest(GradesModelTestCase):
for block_dict in list_of_block_dicts:
block_dict['locator'] = unicode(block_dict['locator']) # BlockUsageLocator is not json-serializable
expected_data = {
'blocks': [{'locator': unicode(self.record_a.locator), 'max_score': 10, 'weight': 1}],
'blocks': [{
'locator': unicode(self.record_a.locator),
'raw_possible': 10,
'weight': 1,
'graded': self.record_a.graded,
}],
'course_key': unicode(self.record_a.locator.course_key),
'version': BLOCK_RECORD_LIST_VERSION,
}
......
"""
Test saved subsection grade functionality.
"""
# pylint: disable=protected-access
import ddt
from django.conf import settings
from django.db.utils import DatabaseError
......@@ -116,7 +116,7 @@ class SubsectionGradeFactoryTest(GradeTestBase):
) as mock_create_grade:
with patch(
'lms.djangoapps.grades.new.subsection_grade.SubsectionGradeFactory._get_saved_grade',
wraps=self.subsection_grade_factory._get_saved_grade # pylint: disable=protected-access
wraps=self.subsection_grade_factory._get_saved_grade
) as mock_get_saved_grade:
with self.assertNumQueries(14):
grade_a = self.subsection_grade_factory.create(self.sequence)
......@@ -205,8 +205,8 @@ class SubsectionGradeTest(GradeTestBase):
input_grade.init_from_structure(
self.request.user,
self.course_structure,
self.subsection_grade_factory._scores_client, # pylint: disable=protected-access
self.subsection_grade_factory._submissions_scores, # pylint: disable=protected-access
self.subsection_grade_factory._submissions_scores,
self.subsection_grade_factory._csm_scores,
)
self.assertEqual(PersistentSubsectionGrade.objects.count(), 0)
......@@ -224,8 +224,8 @@ class SubsectionGradeTest(GradeTestBase):
self.request.user,
saved_model,
self.course_structure,
self.subsection_grade_factory._scores_client, # pylint: disable=protected-access
self.subsection_grade_factory._submissions_scores, # pylint: disable=protected-access
self.subsection_grade_factory._submissions_scores,
self.subsection_grade_factory._csm_scores,
)
self.assertEqual(input_grade.url_name, loaded_grade.url_name)
......
......@@ -5,6 +5,7 @@ from contextlib import contextmanager
from mock import patch
from courseware.module_render import get_module
from courseware.model_data import FieldDataCache
from xmodule.graders import ProblemScore
@contextmanager
......@@ -23,7 +24,7 @@ def mock_get_score(earned=0, possible=1):
Mocks the get_score function to return a valid grade.
"""
with patch('lms.djangoapps.grades.new.subsection_grade.get_score') as mock_score:
mock_score.return_value = (earned, possible)
mock_score.return_value = ProblemScore(earned, possible, earned, possible, 1, True, None, None)
yield mock_score
......
......@@ -948,7 +948,7 @@ def upload_problem_grade_report(_xmodule_instance_args, _entry_id, course_id, _t
final_grade = gradeset['percent']
# Only consider graded problems
problem_scores = {unicode(score.module_id): score for score, _ in gradeset['raw_scores'] if score.graded}
problem_scores = {unicode(score.module_id): score for score in gradeset['raw_scores'] if score.graded}
earned_possible_values = list()
for problem_id in problems:
try:
......
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