wmde / Number

Numerical value objects, parsers and formatters
Other
18 stars 6 forks source link

Stop outputting 100 decimal places for quantities #115

Closed thiemowmde closed 6 years ago

thiemowmde commented 6 years ago

Bug: T155910

mariushoch commented 6 years ago

Seems sensible, but will make the results depend on the precision ini setting. In WMF production this has the default value of 14.

I wonder whether you did this on purpose or whether we rather want to define the precision ourselves (which would mean using for example printf('%0.14f', $theFloat), rather than strval).

mariushoch commented 6 years ago

Note: Will probably also fix T187888.

thiemowmde commented 6 years ago

Uh, thanks for bringing this up.

Note that PHPs precision setting is about significant digits. This is a concept that does not exist as a function in PHP (in contrast to JavaScript, which does have toPrecision). There are certainly ways to build such a function, but I wanted to avoid reimplementing something that already exists as a language feature. I intentionally rely on strval because it is the only way I can pull of PHPs build-in rounding to significant digits that fit what IEEE floating point numbers can represent.

For example, printf( '%.30f', 1 / 3 ) outputs 0.333333333333333314829616256247, and printf( '%.30f', 100000 / 3 ) outputs 33333.333333333335758652538061141968. In both cases, there is no fixed amount of meaningful digits after the decimal point, but (about) 16 significant digits that are meaningful.

PHPs precision setting is a little less, but still precise enough to describe the circumference of the earth (roughly 40,000 km) with a precision of 1 μm. That's quite literally a single piece of dust.

Note that reaching higher precisions is always possible by simply providing a string instead of a floating point number! All the code we are talking about here is exclusively about converting IEEE floats to strings.

mariushoch commented 6 years ago

That sounds very well thought through, and definitely precise and robust enough for our purposes :)

Given this, I think it's safe to merge this!