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.

Sign up for GitHub

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

Jump to bottom

Poll rounding error #11867

Open
moritztim opened this issue Mar 20, 2024 · 5 comments · May be fixed by #13554
Open

Poll rounding error #11867

moritztim opened this issue Mar 20, 2024 · 5 comments · May be fixed by #13554
Assignees
@ShGKme
Labels
2. developing bug feature: frontend 🖌️ "Web UI" client feature: polls 📊
Milestone
💚 Next Patch (29)

Comments

@moritztim
Copy link

How to use GitHub

  • Please use the 👍 reaction to show that you are affected by the same issue.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Steps to reproduce

  1. Create a poll
  2. Have a bunch of people vote
  3. Watch the numbers

Expected behaviour

Tell us what should happen

Actual behaviour

iOS
Android
Linux
Desktop
The math is not mathing.
69+30=99

Talk app

I don't currently have access to the server but I can get access if you need me to.

Talk app version: (see apps admin page: /index.php/settings/apps)

Custom Signaling server configured: yes/no and version (see additional admin settings: /index.php/index.php/settings/admin/talk#signaling_server)

Custom TURN server configured: yes/no (see additional admin settings: /index.php/settings/admin/talk#turn_server)

Custom STUN server configured: yes/no (see additional admin settings: /index.php/settings/admin/talk#stun_server)

Browser

Microphone available: n/a

Camera available: n/a

Operating system: Arch Linux, iOS, Android

Browser name: Native App

Browser version: respectively latest versions

Browser log

n/a

Server configuration

I don't currently have access to the server but I can get access if you need me to.

Operating system: Ubuntu/RedHat/...

Web server: Apache/Nginx

Database: MySQL/Maria/SQLite/PostgreSQL

PHP version: 8.0/8.1/8.2

Nextcloud Version: (see admin page)

List of activated apps:

If you have access to your command line run e.g.:
sudo -u www-data php occ app:list
from within your server installation folder

Nextcloud configuration:

If you have access to your command line run e.g.:
sudo -u www-data php occ config:list system
from within your Nextcloud installation folder

Server log (data/nextcloud.log)

Insert your server log here
@nickvergessen
Copy link
Member

nickvergessen commented Mar 20, 2024 •
edited
Loading

Can you tell us the real numbers of the options?

Nvm says 13 votes, so should be:
4 / 13 => 30.7 %
9 / 13 => 69.2 %

The problem is if we use "normal" rounding 2 options with .5 % would both round up, so:

40.5 %
40.5 %
19 %

Sums to 101 % total.

@nickvergessen nickvergessen added this to the 💚 Next Patch (28) milestone Mar 20, 2024
@fancycode
Copy link
Member

You could round all but the last number and use 100 - SumOfOtherNumbers for that.

@moritztim
Copy link
Author

13
It seems like it's not even being rounded, because 11/13 ≈ 0.8462, which, when rounded to two digits would be 0.85, not 0.84

@ShGKme
Copy link
Contributor

ShGKme commented Jul 31, 2024

We don't round it but just show int number indeed

spreed/src/components/PollViewer/PollViewer.vue

Line 340 in df92396

return parseInt(this.poll?.votes['option-' + index] / this.poll?.numVoters * 100)

@ShGKme ShGKme self-assigned this Jul 31, 2024
@ShGKme
Copy link
Contributor

ShGKme commented Jul 31, 2024 •
edited
Loading

You could round all but the last number and use 100 - SumOfOtherNumbers for that.

This makes the result depending on the order of options and may increase an option that was far from new value when there are options with fewer remainders. Like having 70.5% and the last one 10.1%, it rounds 10.1% to 11% rather than rounding 70.5.

I'd say a proper method should result in:

  • sum to be <= 100%
  • sum to be >= 99%
  • if options are equal, better to show sum 99. E.g. 1/3 should be 33% | 33% | 33% rather than 33% | 33% | 34%

I think the largest remainder methods may work here with a small adjustment to respect equal values.

@nickvergessen nickvergessen removed this from the v19.0.9 milestone Sep 12, 2024
@nickvergessen nickvergessen added this to the 💚 Next Patch (29) milestone Sep 12, 2024
@Antreesy Antreesy linked a pull request Oct 15, 2024 that will close this issue
Open
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ShGKme ShGKme

Labels
2. developing bug feature: frontend 🖌️ "Web UI" client feature: polls 📊
Projects
None yet
Milestone
💚 Next Patch (29)
Development

Successfully merging a pull request may close this issue.

fix: use method of largest remainder to count poll votes nextcloud/spreed
6 participants
@nickvergessen @fancycode @ShGKme @DorraJaouad @moritztim @Antreesy