search

mit-cml / appinventor-sources

MIT App Inventor Public Open Source
http://appinventor.mit.edu/appinventor-sources/
Apache License 2.0
1.48k stars 2.07k forks source link

Round Operator Issue #1339

Closed barreeeiroo closed 4 years ago

barreeeiroo commented 6 years ago

As seen here: https://community.makeroid.io/t/the-round-operation-bug/9144/8?u=barreeeiroo

If you input 2.5 to the round block in Math category, it outputs 2, while the correct output should be 3

ewpatton commented 6 years ago

Technically, this is not a bug. This is a rounding method often referred to as statistician's or banker's rounding, where a tie is rounded to the nearest even number, and it is the method of rounding specified in R6RS. This method is also used for IEEE 754 floating point mathematics when required to round the result of a computation. See here for more information on the different possible rounding approaches.

That being said, I expect that many people will run into this issue where they would normally expect .5 to always round up to the next largest integer. We may want to consider the following improvement:

  1. Introduce a new yail-round-normal that calls through to Math.round, which uses the round up on ties approach (2.5 becomes 3)
  2. Introduce a round scientific option for the unary math blocks that generates yail-round calls
  3. Make round generate calls to yail-round-normal instead of yail-round
  4. Add an upgrade path that converts all existing blocks from round to round scientific to keep the semantics for existing projects.
BeksOmega commented 4 years ago

I think I've got something for this mostly working, so I'd like to lay claim :D

halatmit commented 4 years ago

Please keep this simple. App Inventor's target users are kids. Introducing blocks or properties that control rounding options seems like a very advanced idea. I'd advocate following R6RS and IEE 754.

ewpatton commented 4 years ago

@BeksOmega We may just want to update the documentation for the block, both in the Markdown and in the tooltip, to explain the type of rounding that's done rather than overwhelm people with different options.

BeksOmega commented 4 years ago

Sounds good to me! o7

BeksOmega commented 4 years ago

@ewpatton Just checked, the docs are already up-to-date. And the tooltip currently reads "Round a number up or down." which I think is ok when compared to the tooltips for ceil and floor: "Rounds the input to the smallest number not less then the input" and "Rounds the input to the largest number not greater then the input" respectively.

Given that, would you still like me to update the tooltip?

ewpatton commented 4 years ago

@BeksOmega Yes, we should amend the tooltip to include a note about rounding numbers ending in .5 to the nearest even integer.