fix nearest-neighbor rounding in Util.toFixed

[pixelzoom]

Behavior of Math.round (and therefore dot.Util.toFixed) is currently:

Math.round(1.5) -> 2, Math.round(-1.5) -> -1.

This is not desirable for PhET sims. The desired behavior is one that treats positive and negative values symmetrically. Take care of this in dot.Util.toFixed.

[pixelzoom]

The desired behavior (which I've implemented in Util.toFixed) is IEEE 754 "Round half away from zero" algorithm, see https://en.wikipedia.org/wiki/Rounding#Round_half_away_from_zero. It treats positive and negative values symmetrically.

It occurred to me that it would be nice to have a round function that behaves this way.

[pixelzoom]

Work completed. I added Util.roundSymmetric, and used it in Util.toFixed. I added documentation about the problem with Math.round, and references to the relevant IEEE 754 specs.

Assigning to @jonathanolson to review.

[pixelzoom]

Summary for developers, which I also sent in an email and Skype:

JavaScript’s Math.round uses “round half up” (https://en.wikipedia.org/wiki/Rounding#Round_half_up), which does not treat positive and negative numbers symmetrically.

Example:

> Math.round( 1.5 )
2
> Math.round( -1.5 )
-1

That behavior is problematic for PhET sims, where we want to treat positive and negative numbers symmetrically.

I’ve added dot.Util.roundSymmetric, which treats positive and negative numbers symmetrically and uses "round half away from zero“ (https://en.wikipedia.org/wiki/Rounding#Round_half_away_from_zero).

Example:

> Util.roundSymmetric( 1.5 )
2
> Util.roundSymmetric( -1.5 )
-2

Util.roundSymmetric is also used in Util.toFixed, so that formatting of numbers will also be treat positive and negative numbers symmetrically.

Example:

> Util.toFixed( 1.56, 1 )
“1.6”
> Util.toFixed( -1.56, 1 )
“-1.6"

Recommendations:

• Don’t use Math.round unless you want “round half up” behavior
• Inspect uses of Math.round in existing sim code, and change to Util.roundSymmetric where appropriate
• Look for inappropriate uses of Math.round during code reviews

[pixelzoom]

Re:

• Look for inappropriate uses of Math.round during code reviews

I've added this section to the Code Review Checklist (https://github.com/phetsims/phet-info/blob/master/code_review_checklist.md):

Common Errors

• Is Math.random used where dot.Util.randomSymmetric should be used? Math.random does not treat positive and negative numbers symmetrically, see fix nearest-neighbor rounding in Util.toFixed #35 (comment)
• Is toFixed used where dot.Util.toFixed or dot.Util.toFixedNumber should be used? JavaScript's toFixed is notoriously buggy, behavior differs depending on browser, because the spec doesn't specify whether to round or floor.

[pixelzoom]

Re:

• Inspect uses of Math.round in existing sim code, and change to Util.roundSymmetric where appropriate

Addressing in phetsims/tasks#275.

[jonathanolson]

Looks good to me.

The only case where it really otherwise deviates seems to be -0:

Math.round( -0 ); // -0
roundSymmetric( -0 ); // 0

Feel free to close if it was just pending on review.

[pixelzoom]

@jonathanolson I purposely did not round -0 to -0 because generally we don't want to display -0 in sims. Do you disagree? Is the issue only with -0, or do you think that -1.2 should be rounded to -0?

[jonathanolson]

I purposely did not round -0 to -0 because generally we don't want to display -0 in sims. Do you disagree? Is the issue only with -0, or do you think that -1.2 should be rounded to -0?

Sounds fine to me, just wanted to note it.

[pixelzoom]

I've noted the -0 behavior in the documentation, 65da3f5

[pixelzoom]

Closing.