Issue Details (XML | Word | Printable)

Key: MDL-12842
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Minor Minor
Assignee: Anthony Borrow
Reporter: Kenneth Newquist
Votes: 8
Watchers: 5
Operations

Add/Edit UI Mockup to this issue
If you were logged in you would be able to see more operations.
Moodle

Gradebook fails to properly calculate score when using "drop X lowest" and "exclude assignment"

Created: 06/Jan/08 04:22 AM   Updated: 25/Sep/08 01:47 PM
Component/s: Gradebook
Affects Version/s: 1.8.3
Fix Version/s: 1.9.1

File Attachments: 1. Zip Archive backup-mdl-12842-20080107-2109.zip (5 kB)
2. Zip Archive backup-mdl-12842-20080107-2225.zip (5 kB)
3. Zip Archive backup-mdl-12842p-20080223-0915.zip (6 kB)
4. Zip Archive backup-ts101-20080109-1104.zip (5 kB)
5. Zip Archive backup-ts101-20080109-1146.zip (5 kB)
6. File grade_exceptions_fix.diff (1 kB)
7. File MDL-12842-notes.diff (5 kB)
8. File MDL-12842_SampleDataAnalysis.ods (14 kB)
9. File TS101_Grades.ods (3 kB)

Image Attachments:

1. MDL-12842-analysis.png
(155 kB)

2. MDL-12842-applied.png
(133 kB)

3. MDL-12842-gradebook.png
(117 kB)

4. MDL-12842p.png
(131 kB)
Environment: Moodle 1.8.3 (2007021534), MySQl 5.0.27, PHP 5.2.0
Issue Links:
Relates
 

Database: MySQL
Participants: Anthony Borrow and Kenneth Newquist
Security Level: None
QA Assignee: Kenneth Newquist
Resolved date: 04/Jul/08
Affected Branches: MOODLE_18_STABLE
Fixed Branches: MOODLE_19_STABLE


 Description  « Hide
If the Gradebook is configured to drop the lowest grade AND is setup to exclude an assignment for certain students, it will incorrectly calculate the grades for the excluded students. This bug is shared with Gradebook Plus, as documented here:

http://tracker.moodle.org/browse/CONTRIB-221

Recreating the Problem

* Assign three students to a course.
* Create four offline quizzes with a 100 point scale.
* Create one offline extra credit assignment with a 100 point scale.
* Give everyone in the first event a 25/100.
* For the next three events, give them 100 grades.
* Leave the last event blank; don't given anyone grades.
* Go to Set Perfences and Turn on Advanced Feature

This will cause the gradebook to add the five quizzes together and divide by 5. The point total is

325/500 because the fifth quiz hasn't been taken and has a score of 0. The score is 65%.

If we go to "Set weights" and set "Drop X Lowest" to 1, then the point total becomes 325/400 as number

of quizzes is reduced by one and the maximum score drops from 500 to 400. The score is 81.25% because we

dropped the lowest grade. Note that the lowest grade in this case is the 0 in extra credit NOT the 25 on

Offline #1 as might first be expected.

Because students will only receive a grade if they participate in it, we want to exclude the Extra

Credit results unless someone has a grade. To do this, we use "Grade Exceptions" to omit the students

with no grade from the Extra Credit assignment.

***This does not have the effect we were expecting for the excluded students.***

There is a total of 400 points for the 5 assignments, because we are chosing to drop the lowest grade.

However, the lowest grade (the 25 from Offline Quiz #1) is still being included in the total, making the

student's grade 325/400 when it *should* be 300/400.

At the same time, the Gradebook is still dividing the student's score as though it had dropped the

lowest grade AND excluded the Extra Credit column -- it divides 325 by 3. That gives us a percentage

grade of 108.33.

If we assign a grade to the extra credit assignment, say a 100/100, then everything works properly: the

lowest grade is dropped and we're given a total of 400 out of 400 points, yielding our student a 100%

score.

The remaining students still have wonky grades however.

As a work around, We can assign one of them a score of 0/100 for the extra credit assignmnet continue to

exclude the assignment. If we do this, the "total" remains 400, but the student's grade becomes 300.

Moodle then correctly excludes the Extra Credit assignment and drops the lowest grade (the "25" for

Offline Quiz #1). This yields us a grade of 100% as well.

The student we didn't change -- which still has no value for the excluded assignment -- still has a

grade of 108.33%

This is not a problem if we remove the "Drop X Lowest Grade" option. In this case, the excluded grades

are calculated correctly regardless of whether they have a 0 value or not.


 All   Comments   Change History   Version Control      Sort Order: Ascending order - Click to sort in descending order
Anthony Borrow added a comment - 08/Jan/08 05:15 AM
Ken - It might be helpful if you uploaded the zip file of a course backup here. Then we could just restore it. I'll set things up and give that a shot. Peace - Anthony

Anthony Borrow added a comment - 08/Jan/08 12:08 PM
Ken - Seems a bit complicated. I did not exactly create the sequence as you did. I went to the grade book first and had already turned advanced grading on. I agree that things are not being handled the way we might suspect; however, it might help to have some variety in the scores to make sure that the lowest non-extra-credit grade is being dropped. As I understand it, we should total the points/marks earned (not including extra-credit), drop the x number of grades from that set, then add any extra-credit points and divide by the total number of available or potential points for all items that are not extra-credit. I'm going to upload the version you suggested and then go back and put some diversity into the numbers so that we can play with them. I would want to verify that the lowest is dropped regardless of which assignment it is and that the total possible points works for each student as well. I'll try to keep the numbers somewhat even to make the manual math checking somewhat easier. Peace - Anthony

Anthony Borrow added a comment - 08/Jan/08 12:11 PM
backup of 1.8 course with 5 assignments similar to how Ken describes

Anthony Borrow added a comment - 08/Jan/08 01:28 PM
Here's a secondary course backup with more complicated data that hopefully exposes some of the complexities of what we are trying to accomplish (at least as much as I understand it).

Anthony Borrow added a comment - 08/Jan/08 01:30 PM
Here is an OpenOffice spreadsheet analysis of the data which highlights some of the various complexities in doing the desired analysis. Let me know if I need to upload an excel version or csv version for folks. Peace - Anthony

Anthony Borrow added a comment - 08/Jan/08 01:33 PM
Screenshot of MDL-12842 gradebook with complex data

Anthony Borrow added a comment - 08/Jan/08 01:34 PM
screenshot of openoffice spreadsheet analyzing the data

Kenneth Newquist added a comment - 10/Jan/08 01:12 AM
Backup of example course using the scenario I described in the initial bug report.

Kenneth Newquist added a comment - 10/Jan/08 01:49 AM
Thanks for looking into this Anthony. I should haven't confused the issue by calling the 5th column "extra credit" – it's better to refer to it as "optional grade". It should be included in the total average, not added on after averaging the regular grades.

The process we're replicating:

  • Exclude a column (don't include its grade in calculations at all).
  • Drop the lowest grade.
  • Add the remaining grades and divide by number of remaining grades.

What should be happening is:

Grade 1: 25
Grade 2: 100
Grade 3: 100
Grade 4: 100
Optional Grade 5: null (not included)

Drop 25, added 100+100+100. Divide the sum (300) by the number of columns (3) and get 100. This results in a percentage score of 100%.

Instead what appears to be happening is that it is dropping Grade 5 (the optional, excluded grade), adding 25+100+100+100, getting a sum of 325 and dividing that by 3. This results in a percentage score of 108.33%.

It knows the right number of columns to divide by (because we're dropping the lowest grade) but it's still including that lowest grade column in its math.

If we vary the column the lowest score appears in (e.g. moving Jane's lowest score to Grade #2) the problem remains.

If we change the value of the Grade to 50, the problem remains (but the percentage average is now 116.67%

I have attached two course backups to this tracker:

1. backup-ts101-20080109-1104.zip: An exact replica of the problem I describe.
2. backup-ts101-20080109-1146.zip: A modified version of the problem moving Jane's lowest grade to Assignment #2 and changing it's value to 50.

I've also uploaded an ODS export of the second example.


Kenneth Newquist added a comment - 10/Jan/08 01:49 AM
A modified version of my earlier example, with Jane's lowest grade moved to Assignment #2 and changed to 50.

Kenneth Newquist added a comment - 10/Jan/08 01:50 AM
An ODS export of my second example.

Kenneth Newquist added a comment - 11/Jan/08 05:52 AM
Charles Fulton at Kalamazoo came up with a potential fix for this at a Moodle Hack/Doc Fest we just hosted. I'm posting a diff with the fix. The problem is at line 362:

$grade = $grades_by_student["$exception->userid"]["$exception->catname"]["$assgn->name"]['grade'];
if (isset($grade)) {

if grade isn't set, and the lowest is set to be discounted, the user point total is kept but not the overall total. To fix it, we add this:

$grade = (empty($grade)) ? 0 : $grade;

We've tested it on two development instances, and it remedied the problem for us.


Kenneth Newquist added a comment - 11/Jan/08 05:52 AM
The fix for the "drop x lowest and exclude assignment" bug.

Anthony Borrow added a comment - 17/Jan/08 12:59 PM
Kenneth - My apologies for not being able to test your patches yet and really evaluate this. Yu is expected back at Moodle HQs soon but has been away. I too have been traveling and will leave tomorrow for Mexico so I will be disappearing. I just wanted you to know that this has not gone unnoticed and when I return I will try to make it one of my priorities after catching up from the massive number of emails that will undoubtedly be in my inbox. Peace - Anthony

Anthony Borrow added a comment - 21/Feb/08 05:05 AM
Kenneth - I want to take some time and actually test the patch to ensure that what we think is happening is in fact happening. The case scenario that you provided initially on the bug does not provide enough differentiation. I think we should have each assignment have a different worth so that we can ensure that the lowest is in fact being dropped. I understand that you are not concerned here with extra-credit but simply the exclusion of assignments. I want to make sure that we are excluding not just 100 points but the correct assignment. Can we use my more complicated example and ignore extra-credit and assume an "optional assignment". Peace - Anthony

Kenneth Newquist added a comment - 21/Feb/08 11:26 PM
Sure, I can test this out ASAP.

Anthony Borrow added a comment - 24/Feb/08 02:02 AM
Ken - I'm sorry for being such a dud but I want to make sure we get this right and that I understand what it means to drop the lowest grade. Are we looking at the least number of points or the lowest percentage for a given a student? I would think that it should be the lowest percentage not the grade with the least number of points. This does not matter when all of the assignments are weighted equally; however, when they are not it could become significant. Peace - Anthony

Anthony Borrow added a comment - 24/Feb/08 02:38 AM
Ken - Attached is a course backup that I would like to use as a way of discussing this issue. Could you use this data and let me know what grades you think we should have for each of the five students? I'll attach a screenshot of the data.

I got to the first student and became confused. There are obviously a 120 points; however, if we excluded one of those from the grading [because it is an optional assignment] then do we drop the lowest of the non-excluded grades? If so the total would be 115 points and not a 120 and the possible points would be 135 since the excluded was optional. So I would say that the first students grade shoulld be 115/135.

Student2 (Charlie) never turned in the "optional" assignment (A5) so it is ungraded; however, he is not excluded from it (for whatever reason). So he has 200 possible points and then we remove the lowest grade which is A5. So Charlies has 125 of the possible 150 points - 125/150.

The third student, third assignment poses an interesting scenario for dropping the lowest and demonstrates the issue with lowest percentage or lowest points. If you drop the lowest points you will be dropping an assignment the student scored 100% on. I would student 3 should be 170/175.

Student4 (Gabiel) - Did the optional assignment (A5) so poorly as to merit a grade of zero. Gabiel also earned a zero on A4. Which assignment do we drop? If you drop A5 then Gabiel has 20/150; however if A4 it is 20/100. I would say we ought to drop the one worth the most points which is A4. So Gabiel would have 20/100.

Student5 (Ignacio) - Was not required to do A5 so it is excluded. Then we drop the lowest grade (A4 or A3? - one is zero and one is blank, again I say the one worth the most points so A4). So I think Ignacio's grade would be 25/50.

Let me know your thoughts on this data.

Peace - Anthony


Anthony Borrow added a comment - 24/Feb/08 02:39 AM
Here is a screenshot of the gradebook for the data described in the backup. Peace - Anthony

Anthony Borrow added a comment - 24/Feb/08 02:48 AM
Here is the same data with the patch applied. The more I look at the data the more I get confused.

Anthony Borrow added a comment - 24/Feb/08 04:53 AM
I tried importing the latest backup into Moodle 1.9. The individual exclusions did not restore but there were errors so I will file another report on that. Despite the complexity of the new UI for the gradebook, it is able to handle these scenarios pretty well. I was able, with one exception that seemed strange (material for another bug report), to get the calculations that I would have expected. The exception was with student5 (Ignacio). Depending on whether A3 was 0 and A4 was blank or A3 blank and A4 0 would cause the calculation to come out differently which is not what I would expect. Otherwise, 1.9 looks pretty good IMHO. Peace - Anthony

Anthony Borrow added a comment - 23/Mar/08 11:26 AM
Ken - Have you had an opportunity to work with the restore file attached to this issue? I would like to get this issue resolved in 1.8 core gradebook and GBPv2 but I want to ensure that we are not introducing a bug in the process so I want to double check a variety of scenarios to make sure that it is behaving consistently. Thanks for your help. Peace - Anthony

Anthony Borrow added a comment - 27/Mar/08 05:46 AM
Ken - I am not convinced that the patch addresses the issue. I continue to get the same number of total points. I'm going to setup a couple of test sites to look at this more carefully to make sure we are using the same examples. Peace - Anthony

Anthony Borrow added a comment - 27/Mar/08 07:24 AM
Ken - I have created two test sites for this issue (one with the patch and one without). The sites are located at http://www.jesuitscholar.com/MDL12842 and http://www.jesuitscholar.com/MDL12842P. I setup separate cookies for them so there should be no conflict in being logged into both at the same time within the same browser. The username and password is admin. I have uploaded the case scenarios that I would like to use for this. If we could come to an agreement as to the anticipated results I think we will be on our way to resolving this. I have debugging set to all so that I can pickup whatever other issues may emerge in the process.

Using that data, let's see if we agree on the desired results.

Student1 - 115/135 (exclude #5, drop #2) - straight forward, exclude #5, drop lowest #2
Student2 - 125/150 (exclude nothing, drop #5) - drop #5 since it is null = 0 and not excluded
Student3 - 170/175 (exclude nothing, drop #3) - drop #3 because percentage is lower than #1
Student4 - 20/100 (exclude nothing, drop #4) - percentage is the same (4 and 5) but impact on grade is higher
Student5 - 25/50 (exclude 5, drop #4) - percentage is the same(3 and 4) but impact on grade is higher

I suspect that this will take a major overhaul of the function used to calculate the grades. Before I dive into I want to make sure we agree on the goal.

Peace - Anthony


Anthony Borrow added a comment - 27/Mar/08 07:44 AM
Just to document a little of what I'm thinking (keep in mind I'm not looking at code at the moment but simply trying to formulate in my mind the best way to approach this.In keeping with 1.8 default behavior, nulls will be treated as zeros.

For each category
For each grade_item
if excluded item (do not add to points and possible points)
otherwise keep a running total of points and possible points
Sort grade_item array by percentage (ASC) and possible points (DESC) so we know which ones count more
For however items are to be dropped subtract points and possible points from running totals

In that way excluded grades are never included and then dropped grades are taken out based on the lowest percentage, max points.

Does that make sense?


Kenneth Newquist added a comment - 27/Mar/08 09:40 AM
Anthony,

To piggyback on your comment, in addition to those grades/max scores, we expect these percentages:

115/135 85.18
125/150 83.33
170/175 97.14
20/100 20.00
25/50 50.00

Based on the logic you've outlined, these numbers are what you'd expect to see.

As far as the preference goes as to which to discard (greater impact on grade or lower impact on grade) I can see people arguing it both ways; if you go with the later though, you're effectively discarding tests in favor of quizzes, right?

I'll bounce it off my instructional technologists tomorrow and see if they have any practical use cases that might inform this. Whatever way this goes though, I think we need to update the documentation to spell out what the logic is here; that the preferred approach is not to have grades of differing max scores, and that if you do use differing values, be aware that the X is dropped in favor of Y because of Z.

As for the patch, I concur; it is not fixing the problem; we can confirm this by matching the actual percentages with the expected ones above.

For this course, we have a total possible score (assuming the student completed all assignments) of 200.

For Student #1 (Baker, Adam), Assignment #5 is excluded. This drops his total possible score to 150. His cumulative grade, without dropping the lowest score, is 120 out of 150.

We drop his lowest grade, which assuming we're using percentages to calculate lowest grade, should be Assignment #2 (5/15). This lowers his total possible score to 135. His cumulative grade, dropping the lowest percentage score, is 115 out of 135. This should out to 85.14%.

What the patched version appears to be doing is using Assignment #2 (5/15) as the lowest grade (giving us 5) but Assignment #1 (10/10) as the lowest possible score (give us a 10).

The hidden math is a cumulative grade of 115, but a total possible score of 140. 115/140 gives us 82.14%, which is the value we see on screen.

This did not show up in my version, because all the assignments were worth the same amount of points (100) so it didn't matter which assignment it considered to have the lowest possible max score.


Anthony Borrow added a comment - 27/Mar/08 11:38 AM
Ken - The only discrepancy I see in what you said is between the calculation of student1's score. I calculate 115/135 as being 85.185 which I think should round to 85.19. On the logic I think we are in agreement. With respect to the question of dropping the lowest score, I think it should always be the score that will have the greatest impact on the grade is percentage wise the lowest score. Keep in mind that this is only within the category. So if a teacher wants to drop the lowest test they can. If they keep each on worth the same amount (i.e. 100 points) then no worries; however, if they make one worth 200 points and the student happens to get the lowest grade on that then good for the student. In other words, teachers need to understand what they are doing when setting up dropping and your recommendation to keep things equal is a good one. As long as we can explain the logic to them I think we are in good shape. I'm happy I can understand the logic after looking at all these numbers! I think the case scenarios cover most of the possible situations we might find. Now to make the code do what we want it to do. Thanks for sticking with me on this issue. Peace - Anthony

Anthony Borrow added a comment - 27/Mar/08 02:05 PM
Ken - I have gone through and have a much better understanding of how the grades are being calculated. It seems to me that there is some faulty logic in the procedure. The first time totals and points are calculated for the students it is conditional upon there not being drops:

if ($main_category['stats']['drop'] != 0) { // I think the exceptions should be handled not only first but separtely from drops

Later when re-calculating after removing excluded and dropped grades the logic begins with:

if (isset($exceptions) && $exceptions) {

and then handles the drop cases within that if statement.

I'm going to need to print out the function and do some major refactoring. If I can get away with a minor fix I will; however, I am going to do so with great care which means I will need more time. It may well be June before I really get a chance to work on this again.

If someone is able to provide a patch before then I would be happy to look at it. I've got some good notes which should help me along when I do get back to it but I wanted to let you know that it may be a while.

Peace - Anthony


Anthony Borrow added a comment - 27/Mar/08 02:22 PM
uploading a diff file containing some documentation and spacing cleanup I did as I was reviewing the code

Anthony Borrow added a comment - 04/Jul/08 06:54 AM
Ken - I am marking this as fixed for Moodle 1.9. I don't think it is worth trying to sort it out for 1.8; however, if there is strong sentiment or need for this then I am willing to give it a crack. Peace - Anthony