fix: increase grading rounding precision to avoid incorrect grades

[Agrendalath]

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

OSPR-5819

Sandbox

https://pr27788.sandbox.opencraft.hosting/
Direct link to the progress page (you can log in as staff).

Testing instructions

1. Import this course in Studio.
2. Complete two units from there.
3. Go to the Progress page 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%):

1. In the first subsection, users can get 2 out of 3 points (66.67%).
2. 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

• @arjunsinghy96
• edX reviewer[s] TBD

Other information

Private-ref: BB-4210
Product Review: openedx/platform-roadmap#238
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]

[openedx-webhooks]

Thanks for the pull request, @Agrendalath!

What's next?

Please work through the following steps to get your changes ready for engineering review:

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @openedx/wg-maintenance-edx-platform. Tag them in a comment and let them know that your changes are ready for review.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

[arjunsinghy96]

👍 @Agrendalath LGTM.

• I tested this: Scores are displayed with more precision on Progress page graph.
• I read through the code
• I checked for accessibility issues
• Includes documentation
• N/A I made sure any change in configuration variables is reflected in the corresponding client's configuration-secure repository.

[Agrendalath]

@natabene, this is ready for your review.

[natabene]

@Agrendalath Thank you for your contribution.

[arch-bom-gocd-alerts]

📣 💥 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 2e33565 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).

[edx-status-bot]

Your PR has finished running tests. There were no failures.

[nizarmah]

Hello @natabene, would this change require a product review or is it suitable for a core contributor review?

[natabene]

@nizarmah Let me check and get back to you asap.

[natabene]

@nizarmah Thanks for asking, we considered this and will need to do a product review first.

[Agrendalath]

@natabene, just checking on this PR.

[natabene]

Sorry, no news yet.

[ProductRyan]

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?

[Agrendalath]

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).

[Agrendalath]

@ProductRyan, @natabene, just a friendly reminder that this is still waiting for the product review.

[Agrendalath]

@mphilbrick211, sure thing. Done.

[Agrendalath]

@gabor-boros, I see "This site is currently being provisioned. Please try again in a few minutes." when I try to access the sandbox.

[gabor-boros]

@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.

[gabor-boros]

@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.)

[Agrendalath]

@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.".

[open-craft-grove]

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

[open-craft-grove]

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

[open-craft-grove]

Sandbox destroy request received.

[feanil]

@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.

[Agrendalath]

@feanil, thank you for checking. We're definitely interested in merging this. Please see my last comment in the product review thread.

[feanil]

Sounds good , thanks I'll watch the product review thread and see what I can do to help unblock that.

[feanil]

Approving since Jenna has signed off on this from the product side: openedx/platform-roadmap#238 (comment)

[Agrendalath]

Update: I'll merge this once openedx/frontend-app-learning#1397 is approved.