Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed weight ignore issue on grade calculation #97

Closed

Conversation

amir-qayyum-khan
Copy link

HI

Background:

  • Add Grade to assignment.
  • Expected behaviour: Grader will offer a grade between 0 and 100, and should be multiplied by weight to get % of full problem.
  • Actual behaviour: Grade is a grade between 0 and 100. Problem weight seems to be ignored.

Done in this PR

In this PR i am using weight to calculate grade percentage. Applied this logic on two points. Images are attached to demonstrate.

screen shot 2015-06-04 at 6 18 57 pm

screen shot 2015-06-04 at 6 18 54 pm

screen shot 2015-06-04 at 6 18 41 pm

Issue: #95
@pdpinch @carsongee

@@ -13,7 +13,7 @@
<p>No file has been uploaded.</p>
<% } %>
<% if (graded) { %>
<p>Your score is <%= graded.score %> / <%= max_score %><br/>
<p>Your score is <%= (graded.score / max_score) * weight %> / <%= weight %><br/>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pdpinch @carsongee If weight is Zero (0), it will show grade 0 / 0 is this ok?

zero

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sfrucht do you know what standard edX behavior is for a problem with a weight of 0 ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pdpinch A problem with weight of 0 is ungraded and does not show in the problem. See attached.
prob_wt_1
prob_wt_0

@pdpinch
Copy link
Member

pdpinch commented Jun 4, 2015

@bdero Not a top priority, but can we get a sandbox with this branch of SGA, so @sfrucht can take a look at it?

Thanks

@pwilkins
Copy link

This looks good to me, it's ready to merge. Shall I hold off merging for Shira's review?

@pwilkins
Copy link

I was wrong, there is still an outstanding issue. PeterP commented that score should only be displayed when weight > 0. Currently, when weight == 0, score displays as your score is 0/0 which isn't expected edX behavior.

@amir-qayyum-khan
Copy link
Author

Issue fixed @pwilkins

@bdero
Copy link

bdero commented Jun 17, 2015

@pwilkins @pdpinch This is what we need on a sandbox, right?

@pwilkins
Copy link

@bdero Correct. This is what we want Shira to review.

@pdpinch
Copy link
Member

pdpinch commented Jun 18, 2015

Shira took a look at this. It's working in the student view (courseware), but not in the progress page. It's probably not working in grade export either, although that wasn't tested.

I think we have to look at how we're using the submissions_api to record student scores and see if we need to pass the weight. See https://github.com/amir-qayyum-khan/edx-sga/blob/mx_score_weight_fix/edx_sga/sga.py#L548

Hmm. Looking at http://edx-submissions-api.readthedocs.org/en/latest/api.html#submissions.api.set_score I don't see a way to pass the weight. This may require a posting to edx-code.

@amir-qayyum-khan
Copy link
Author

@pdpinch instead of sending score and max score pair (https://github.com/amir-qayyum-khan/edx-sga/blob/mx_score_weight_fix/edx_sga/sga.py#L548)
what if we send student weightage and weight pair. In this scenario if weight is 0 edX should handle.

@pdpinch
Copy link
Member

pdpinch commented Jun 22, 2015

I spoke with Dave Ormsbee who explained that the weight is not passed to the submission_api. Instead weight is a field set on the xblock. When the progress page is rendered it uses the weight to calculate grades.

I think the next step is to take a look at the progress page code and try to determine why it isn't using the correct weight.

@amir-qayyum-khan
Copy link
Author

Weight ignorance issue in courseware/grade.py

screen shot 2015-06-25 at 4 38 16 pm

If we remove grade fetch from cache i.e disable lines https://github.com/edx/edx-platform/blob/master/lms/djangoapps/courseware/grades.py#L438-440

screen shot 2015-06-25 at 4 36 47 pm

screen shot 2015-06-25 at 4 36 56 pm

@pdpinch @pwilkins There is issue on edX platform side which is still present.
I only fixed issue on SGA side, The fix is in this PR. So what should I do next?

@pdpinch
Copy link
Member

pdpinch commented Jun 26, 2015

@amir-qayyum-khan I spoke again with Dave Ormsbee at edX and he confirmed the bug you've discovered. It should be fixed in https://github.com/edx/edx-platform/pull/7288 which hasn't merged yet.

@pdpinch
Copy link
Member

pdpinch commented Jun 30, 2015

@amir-qayyum-khan can you take a look at the changes in https://github.com/edx/edx-platform/blob/ormsbee/grade_query_caching/lms/djangoapps/courseware/grades.py and see if it fixes the weight caching issue?

@amir-qayyum-khan
Copy link
Author

@pdpinch it is not fix for SGA see lines.

@pwilkins
Copy link

pwilkins commented Jul 9, 2015

@amir-qayyum-khan Since David Ormsbee's PR won't fix this, what do you propose as a next step?
@pdpinch

@pdpinch
Copy link
Member

pdpinch commented Jul 13, 2015

If the current state of the PR is limited to the student view (i.e. courseware) then we can merge it.

The fix to the progress page will require fixes at edX. For what it's worth, ORA2 problems (peer assessment) have the same problem as SGA currently.

@tssheth
Copy link

tssheth commented Jul 23, 2015

When I grade a submitted file for an SGA with no weight, I do not see the grade I entered after I submit it. The instructor sees a blank space in the "Grade" column of the "Grade Submissions" panel. The student does not see any mention of a grade, only the comment that the instructor made.

Instructor:
screen shot 2015-07-23 at 1 39 22 pm

screen shot 2015-07-23 at 1 39 31 pm

Student:
screen shot 2015-07-23 at 1 40 55 pm

@pdpinch
Copy link
Member

pdpinch commented Jul 23, 2015

I'd like to go ahead and merge this as it fixes half of the problem -- displaying the grade/weight in the courseware view.

We can open a new issue about the progress page issue.

On Jul 9, 2015, at 9:51 AM, Peter Wilkins [email protected] wrote:

@amir-qayyum-khan https://github.com/amir-qayyum-khan Since David Ormsbee's won't fix this, what do you propose as a next step?
@pdpinch https://github.com/pdpinch

Reply to this email directly or view it on GitHub #97 (comment).

@pdpinch
Copy link
Member

pdpinch commented Jul 23, 2015

I think this is consistent with other XBlocks

The thinking is that if the problem is worth 0 points, we shouldn't be telling the learner how many points they got. Also, it looks odd to display something like "49 / 0"

On Jul 23, 2015, at 1:48 PM, Tej Sheth [email protected] wrote:

When I grade a submitted file for an SGA with no weight, I do not see the grade I entered after I submit it. The instructor sees a blank space in the "Grade" column of the "Grade Submissions" panel. The student does not see any mention of a grade, only the comment that the instructor made.

https://cloud.githubusercontent.com/assets/8117756/8857693/4102829a-3141-11e5-9519-aae001a386d6.png
https://cloud.githubusercontent.com/assets/8117756/8857696/48355614-3141-11e5-9382-5a70a5708747.png
https://cloud.githubusercontent.com/assets/8117756/8857699/4b5deb1c-3141-11e5-9bb1-7e79bcf5f9ac.png

Reply to this email directly or view it on GitHub #97 (comment).

@codecov-io
Copy link

codecov-io commented Apr 4, 2018

Codecov Report

Merging #97 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #97      +/-   ##
==========================================
+ Coverage   92.58%   92.59%   +0.01%     
==========================================
  Files          16       16              
  Lines        1550     1553       +3     
  Branches       94       94              
==========================================
+ Hits         1435     1438       +3     
  Misses        106      106              
  Partials        9        9
Impacted Files Coverage Δ
edx_sga/sga.py 95.32% <100%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0cb0e49...7273cd5. Read the comment docs.

@amir-qayyum-khan
Copy link
Author

close in favour of #242

@amir-qayyum-khan
Copy link
Author

close in favour of #242

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants