edx-platform
edx-platform copied to clipboard
openedx
fix: increase grading rounding precision to avoid incorrect grades
Enabling the rounding in #16837 has been causing noticeable (up to 1 percentage point) differences between non-rounded subsection grades and a total grade for a course. This increases the grade precision to reduce the negative implications of double rounding.
Jira
Sandbox
https://pr27788.sandbox.opencraft.hosting/ Direct link to the progress page (you can log in as staff).
Testing instructions
- Import this course in Studio.
- Complete two units from there.
- Go to the
Progresspage and see that the score without this PR is 35%. After this change, it should be 34%.
Explanation
This course contains two subsections graded as Homework (weight: 75%):
- In the first subsection, users can get 2 out of 3 points (66.67%).
- In the second one, users can get 1 out of 4 points (25%).
In the current approach, the grades of subsections are rounded. Therefore, in this case, we're getting (67% + 25%) / 2 / 4 * 3 (/ 4 * 3 is because of the weight (75%)), which gives us 34.5. This is rounded up to 35%.
This PR changes these calculations to (66.67% + 25%) / 2 / 4 * 3, which returns 34.37625, which is then rounded down to 34%.
Deadline
None.
Reviewers
- [x] @arjunsinghy96
- [ ] edX reviewer[s] TBD
Other information
Private-ref: BB-4210
Settings
Tutor requirements
git+https://github.com/overhangio/tutor.git@nightly
git+https://github.com/overhangio/tutor-discovery.git@nightly
git+https://github.com/overhangio/tutor-ecommerce.git@nightly
git+https://github.com/overhangio/tutor-xqueue.git@nightly
git+https://github.com/overhangio/tutor-forum.git@nightly
git+https://gitlab.com/opencraft/dev/[email protected]
git+https://github.com/hastexo/[email protected]
Thanks for the pull request, @Agrendalath! Please note that it may take us up to several weeks or months to complete a review and merge your PR.
Feel free to add as much of the following information to the ticket as you can:
- supporting documentation
- Open edX discussion forum threads
- timeline information ("this must be merged by XX date", and why that is)
- partner information ("this is a course on edx.org")
- any other information that can help Product understand the context for the PR
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.
This is currently a draft pull request. When it is ready for our review and all tests are green, click "Ready for Review", or remove "WIP" from the title, as appropriate.
📣 💥 Heads-up: You must either rebase onto master or merge master into your branch to avoid breaking the build.
We recently removed diff-quality and introduced lint-amnesty. This means that the automated quality check that has run on your branch doesn't work the same way it will on master. If you have introduced any quality failures, they might pass on the PR but then break the build on master.
This branch has been detected to not have commit 2e335653 as an ancestor. Here's how to see for yourself:
git merge-base --is-ancestor 2e335653 agrendalath/bb-4210-increase_grading_precision && echo "You're all set" || echo "Please rebase onto master or merge master to your branch"
If you have any questions, please reach out to the Architecture team (either #edx-shared-architecture on Open edX Slack or #architecture on edX internal).
Hello @natabene, would this change require a product review or is it suitable for a core contributor review?
@nizarmah Thanks for asking, we considered this and will need to do a product review first.
Hey @Agrendalath - I'm the new 2U Product Manager working on grading. Apologies on the delay in response here.
Can you confirm my understanding of this change?
It appears that this will increase the accuracy, of the rounding that is already in place, up to 4 decimal places.
Is that correct? are there any other changes that I'm not catching?
Hi @ProductRyan, thank you for checking. That's correct - the subsection grades calculated by the platform are inaccurate because of the double rounding. Therefore, we want to increase the rounding precision to produce correct results.
This change will not affect existing persistent grades as long as they are not re-generated. Otherwise, this can cause up to a 1 percentage point difference in the results (on a rare occasion, like the one described in the PR).
@ProductRyan, @natabene, just a friendly reminder that this is still waiting for the product review.
Flagging for you @ProductRyan and @jmakowski1123 - looks like this is waiting on Product review.
Hi @jmakowski1123 and @ProductRyan - flagging this. Looks like it still needs product review.
Status update: 1/31 - On hold for review across platform for consistent UI. Timeline TBD.
Since this ticket requires deeper edX product input, I've created a Product Feature ticket here to track continued product communications and feedback.
Hi @Agrendalath! While this is with product in review, would you mind re-running the tests? We've had a couple new ones pop up ("shellcheck"). Thanks!
@Agrendalath if you need, you can access the sandbox at https://pr-27788-0e7f8a.staging.do.opencraft.hosting/
@gabor-boros, I see "This site is currently being provisioned. Please try again in a few minutes." when I try to access the sandbox.
@Agrendalath Ah, in the meantime I had to reprovision the whole cluster, so the instance got destroyed. I'm going to quickly recreate the instance.
@Agrendalath the sandbox is up and running but forum won't work as of now. -- I need to fix something before. If you update the branch, please trigger a pipeline manually as I disabled the PR watcher now. (i.e. do a commit with [AutoDeploy][Update] pr-27788-0e7f8a|2 message for the grove-stage-digitalocean repo.)
@gabor-boros, the MFEs also seem to be down - when I try to sign in, I get "The page you're looking for is unavailable or there's an error in the URL. Please check the URL and try again.".
Sandbox deployment successful.
Sandbox LMS is available at pr-27788-139931.staging.do.opencraft.hosting Sandbox Studio is available at studio.pr-27788-139931.staging.do.opencraft.hosting
Sandbox deployment successful.
Sandbox LMS is available at pr-27788-139931.staging.do.opencraft.hosting Sandbox Studio is available at studio.pr-27788-139931.staging.do.opencraft.hosting
@Agrendalath I'm going through the old PRs, do you still want to pursue this PR or should we close this for now? If you're still interested I can try to get some eyes on this that could review this for you.
@feanil, thank you for checking. We're definitely interested in merging this. Please see my last comment in the product review thread.