From 86974893c89a7504a067fb7d6aca932c2acf591b Mon Sep 17 00:00:00 2001 From: Frederic Massart Date: Thu, 9 Oct 2014 15:25:55 +0800 Subject: [PATCH 1/4] MDL-47489 core_grades: Adjust weights of extra credit items for Natural --- lib/grade/grade_category.php | 56 +++++++++++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php index 889006f993a..8058e190d85 100644 --- a/lib/grade/grade_category.php +++ b/lib/grade/grade_category.php @@ -1067,11 +1067,13 @@ class grade_category extends grade_object { break; case GRADE_AGGREGATE_SUM: // Add up all the items. + $this->load_grade_item(); $num = count($grade_values); $sum = 0; $sumweights = 0; $grademin = 0; $grademax = 0; + $extracredititems = array(); foreach ($grade_values as $itemid => $gradevalue) { // We need to check if the grademax/min was adjusted per user because of excluded items. $usergrademin = $items[$itemid]->grademin; @@ -1083,8 +1085,13 @@ class grade_category extends grade_object { $usergrademax = $grademaxoverrides[$itemid]; } + // Keep track of the extra credit items, we will need them later on. + if ($items[$itemid]->aggregationcoef > 0) { + $extracredititems[$itemid] = $items[$itemid]; + } + // Ignore extra credit and items with a weight of 0. - if ($items[$itemid]->aggregationcoef <= 0 && $items[$itemid]->aggregationcoef2 > 0) { + if (!isset($extracredititems[$itemid]) && $items[$itemid]->aggregationcoef2 > 0) { $grademin += $usergrademin; $grademax += $usergrademax; $sumweights += $items[$itemid]->aggregationcoef2; @@ -1135,11 +1142,58 @@ class grade_category extends grade_object { // We can use our freshly corrected weights below. foreach ($grade_values as $itemid => $gradevalue) { + if (isset($extracredititems[$itemid])) { + // We skip the extra credit items first. + continue; + } $sum += $gradevalue * $userweights[$itemid] * $grademax; if ($weights !== null) { $weights[$itemid] = $userweights[$itemid]; } } + + // No we proceed with the extra credit items. They might have a different final + // weight in case the final grade was bounded. So we need to treat them different. + // Also, as we need to use the bounded_grade() method, we have to inject the + // right values there, and restore them afterwards. + $oldgrademax = $this->grade_item->grademax; + $oldgrademin = $this->grade_item->grademin; + foreach ($grade_values as $itemid => $gradevalue) { + if (!isset($extracredititems[$itemid])) { + continue; + } + $oldsum = $sum; + $weightedgrade = $gradevalue * $userweights[$itemid] * $grademax; + $sum += $weightedgrade; + + // Only go through this when we need to record the weights. + if ($weights !== null) { + if ($grademax <= 0) { + // There are only extra credit items in this category, + // all the weights should be accurate (and be 0). + $weights[$itemid] = $userweights[$itemid]; + continue; + } + + $oldfinalgrade = $this->grade_item->bounded_grade($oldsum); + $newfinalgrade = $this->grade_item->bounded_grade($sum); + $finalgradediff = $newfinalgrade - $oldfinalgrade; + if ($finalgradediff <= 0) { + // This item did not contribute to the category total at all. + $weights[$itemid] = 0; + } else if ($finalgradediff < $weightedgrade) { + // The weight needs to be adjusted because only a portion of the + // extra credit item contributed to the category total. + $weights[$itemid] = $finalgradediff / ($gradevalue * $grademax); + } else { + // The weight was accurate. + $weights[$itemid] = $userweights[$itemid]; + } + } + } + $this->grade_item->grademax = $oldgrademax; + $this->grade_item->grademin = $oldgrademin; + if ($grademax > 0) { $agg_grade = $sum / $grademax; // Re-normalize score. } else { From e8ac04c191c00bee4de539dee6056fd3f0ea4340 Mon Sep 17 00:00:00 2001 From: Frederic Massart Date: Fri, 10 Oct 2014 11:47:58 +0800 Subject: [PATCH 2/4] MDL-47489 core_grades: Adjust weights of extra credit items for Mean --- lib/grade/grade_category.php | 41 +++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php index 8058e190d85..db79b9a3061 100644 --- a/lib/grade/grade_category.php +++ b/lib/grade/grade_category.php @@ -1031,27 +1031,61 @@ class grade_category extends grade_object { break; case GRADE_AGGREGATE_EXTRACREDIT_MEAN: // special average + $this->load_grade_item(); $num = 0; $sum = null; foreach ($grade_values as $itemid=>$grade_value) { - if ($items[$itemid]->aggregationcoef == 0) { $num += 1; $sum += $grade_value; if ($weights !== null) { $weights[$itemid] = 1; } + } + } - } else if ($items[$itemid]->aggregationcoef > 0) { + // Treating the extra credit items separately to get a chance to calculate their effective weights. + foreach ($grade_values as $itemid=>$grade_value) { + if ($items[$itemid]->aggregationcoef > 0) { + $oldsum = $sum; $sum += $items[$itemid]->aggregationcoef * $grade_value; + if ($weights !== null) { - $weights[$itemid] = 1; + if ($num <= 0) { + // The category only contains extra credit items, not setting the weight. + continue; + } + + $oldgrade = $oldsum / $num; + $grade = $sum / $num; + $normoldgrade = grade_grade::standardise_score($oldgrade, 0, 1, $grademin, $grademax); + $normgrade = grade_grade::standardise_score($grade, 0, 1, $grademin, $grademax); + $boundedoldgrade = $this->grade_item->bounded_grade($normoldgrade); + $boundedgrade = $this->grade_item->bounded_grade($normgrade); + + if ($boundedgrade - $boundedoldgrade <= 0) { + // Nothing new was added to the grade. + $weights[$itemid] = 0; + } else if ($boundedgrade < $normgrade) { + // The grade has been bounded, the extra credit item needs to have a different weight. + $gradediff = $boundedgrade - $normoldgrade; + $gradediffnorm = grade_grade::standardise_score($gradediff, $grademin, $grademax, 0, 1); + $weights[$itemid] = $gradediffnorm / $grade_value; + } else { + // Default weighting. + $weights[$itemid] = 1.0 / $num; + } } } } + if ($weights !== null && $num > 0) { foreach ($grade_values as $itemid=>$grade_value) { + if ($items[$itemid]->aggregationcoef > 0) { + // Extra credit weights were already calculated. + continue; + } if ($weights[$itemid]) { $weights[$itemid] = 1.0 / $num; } @@ -1064,6 +1098,7 @@ class grade_category extends grade_object { } else { $agg_grade = $sum / $num; } + break; case GRADE_AGGREGATE_SUM: // Add up all the items. From 8427dc3a23f8d3040980c681ec0ca47e5ad7b857 Mon Sep 17 00:00:00 2001 From: Frederic Massart Date: Fri, 10 Oct 2014 12:42:13 +0800 Subject: [PATCH 3/4] MDL-47489 core_grades: Adjust weights of extra credit items for Simple --- lib/grade/grade_category.php | 60 +++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php index db79b9a3061..ae80b6dffa8 100644 --- a/lib/grade/grade_category.php +++ b/lib/grade/grade_category.php @@ -996,30 +996,82 @@ class grade_category extends grade_object { case GRADE_AGGREGATE_WEIGHTED_MEAN2: // Weighted average of all existing final grades with optional extra credit flag, // weight is the range of grade (usually grademax) + $this->load_grade_item(); $weightsum = 0; $sum = null; foreach ($grade_values as $itemid=>$grade_value) { - $weight = $items[$itemid]->grademax - $items[$itemid]->grademin; + if ($items[$itemid]->aggregationcoef > 0) { + continue; + } + $weight = $items[$itemid]->grademax - $items[$itemid]->grademin; if ($weight <= 0) { continue; } - if ($items[$itemid]->aggregationcoef == 0) { - $weightsum += $weight; - } + $weightsum += $weight; $sum += $weight * $grade_value; } + + // Handle the extra credit items separately to calculate their weight accurately. + foreach ($grade_values as $itemid => $grade_value) { + if ($items[$itemid]->aggregationcoef <= 0) { + continue; + } + + $weight = $items[$itemid]->grademax - $items[$itemid]->grademin; + if ($weight <= 0) { + $weights[$itemid] = 0; + continue; + } + + $oldsum = $sum; + $weightedgrade = $weight * $grade_value; + $sum += $weightedgrade; + + if ($weights !== null) { + if ($weightsum <= 0) { + $weights[$itemid] = 0; + continue; + } + + $oldgrade = $oldsum / $weightsum; + $grade = $sum / $weightsum; + $normoldgrade = grade_grade::standardise_score($oldgrade, 0, 1, $grademin, $grademax); + $normgrade = grade_grade::standardise_score($grade, 0, 1, $grademin, $grademax); + $boundedoldgrade = $this->grade_item->bounded_grade($normoldgrade); + $boundedgrade = $this->grade_item->bounded_grade($normgrade); + + if ($boundedgrade - $boundedoldgrade <= 0) { + // Nothing new was added to the grade. + $weights[$itemid] = 0; + } else if ($boundedgrade < $normgrade) { + // The grade has been bounded, the extra credit item needs to have a different weight. + $gradediff = $boundedgrade - $normoldgrade; + $gradediffnorm = grade_grade::standardise_score($gradediff, $grademin, $grademax, 0, 1); + $weights[$itemid] = $gradediffnorm / $grade_value; + } else { + // Default weighting. + $weights[$itemid] = $weight / $weightsum; + } + } + } + if ($weightsum == 0) { $agg_grade = $sum; // only extra credits } else { $agg_grade = $sum / $weightsum; } + // Record the weights as used. if ($weights !== null) { foreach ($grade_values as $itemid=>$grade_value) { + if ($items[$itemid]->aggregationcoef > 0) { + // Ignore extra credit items, the weights have already been computed. + continue; + } if ($weightsum > 0) { $weight = $items[$itemid]->grademax - $items[$itemid]->grademin; $weights[$itemid] = $weight / $weightsum; From 40f874ce2921d6ba4f0edd9a5bc3dab284451ae2 Mon Sep 17 00:00:00 2001 From: Frederic Massart Date: Fri, 10 Oct 2014 15:56:17 +0800 Subject: [PATCH 4/4] MDL-47489 core_grades: Behat tests covering contributions with extra credit --- ...ade_contribution_with_extra_credit.feature | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 grade/tests/behat/grade_contribution_with_extra_credit.feature diff --git a/grade/tests/behat/grade_contribution_with_extra_credit.feature b/grade/tests/behat/grade_contribution_with_extra_credit.feature new file mode 100644 index 00000000000..88e45d92096 --- /dev/null +++ b/grade/tests/behat/grade_contribution_with_extra_credit.feature @@ -0,0 +1,81 @@ +@core @core_grades @javascript +Feature: Extra credit contributions are normalised when going out of bounds + In order to use extra credit + As a teacher + I need to add some extra credit items. + + Background: + Given the following "courses" exist: + | fullname | shortname | category | groupmode | + | Course 1 | C1 | 0 | 1 | + And the following "users" exist: + | username | firstname | lastname | email | idnumber | + | teacher1 | Teacher | 1 | teacher1@asd.com | t1 | + | student1 | Student | 1 | student1@asd.com | s1 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + And I log in as "admin" + And I set the following administration settings values: + | grade_aggregations_visible | Simple weighted mean of grades,Mean of grades (with extra credits),Natural | + And I am on homepage + And I follow "Course 1" + And I follow "Grades" + And I navigate to "Categories and items" node in "Grade administration > Setup" + And I press "Add grade item" + And I set the following fields to these values: + | Item name | Manual item 1 | + | Maximum grade | 150 | + And I press "Save changes" + And I press "Add grade item" + And I set the following fields to these values: + | Item name | Manual item 2 | + And I press "Save changes" + And I press "Add grade item" + And I set the following fields to these values: + | Item name | Manual item 3 | + And I press "Save changes" + And I press "Add grade item" + And I set the following fields to these values: + | Item name | Manual item 4 | + And I press "Save changes" + And I navigate to "Course grade settings" node in "Grade administration > Setup" + And I set the field "Show weighting" to "Show" + And I set the field "Show contribution to course total" to "Show" + And I press "Save changes" + And I log out + And I log in as "teacher1" + And I follow "Course 1" + And I follow "Grades" + And I turn editing mode on + And I give the grade "80.00" to the user "Student 1" for the grade item "Manual item 1" + And I give the grade "10.00" to the user "Student 1" for the grade item "Manual item 2" + And I give the grade "70.00" to the user "Student 1" for the grade item "Manual item 3" + And I give the grade "90.00" to the user "Student 1" for the grade item "Manual item 4" + And I press "Save changes" + + Scenario Outline: The contribution of extra credit items is normalised + Given I set the field "Grade report" to "Categories and items" + When I set the following settings for grade item "Course 1": + | Aggregation | | + And I set the following settings for grade item "Manual item 2": + | Extra credit | 1 | + And I set the following settings for grade item "Manual item 3": + | Extra credit | 1 | + And I set the following settings for grade item "Manual item 4": + | Extra credit | 1 | + And I set the field "Grade report" to "User report" + And I set the field "Select all or one user" to "Student 1" + Then the following should exist in the "user-grade" table: + | Grade item | Calculated weight | Grade | Contribution to course total | + | Manual item 1 | | 80.00 | | + | Manual item 2 | | 10.00 | | + | Manual item 3 | | 70.00 | | + | Manual item 4 | 0.00 % | 90.00 | 0.00 | + + Examples: + | aggregation | m1w | m1c | m2w | m2c | m3w | m3c | + | Natural | 100.00 % | 80.00 | 66.67 % | 10.00 | 57.14 % | 60.00 | + | Simple weighted mean of grades | 100.00 % | 53.33 | 66.67 % | 6.67 | 57.14 % | 40.00 | + | Mean of grades (with extra credits) | 100.00 % | 53.33 | 100.00 % | 10.00 | 52.38 % | 36.67 |