eobermuhlner/big-math

BigRational toFloat() can fail in a slightly unexpected way

Closed this issue · 4 comments

Consider the following example which shows why the method fails and an alternative (Float.valueOf(x.toBigDecimal().toString())) that works more how I expected, but would fail if x could not be represented as a BigDecimal.
BigRational x = BigRational.valueOf("8.804462619980757911125181749462772084351754080848495898653087767533866267556634");
System.out.println(x.toRationalString());
System.out.println(x.toFloat());
System.out.println(Float.valueOf(x.toBigDecimal().toString()));
System.out.println(new BigDecimal("1000000000000000000000000000000000000000000000000000000000000000000000000000000").floatValue());

I am not sure I understand what you mean with "the method fails".

BigRational.toRationalString() is a representation of the exact rational number without loss of accuracy.
From the javadoc:

Returns the string representation of this rational number in the form "numerator/denominator".

BigRational.toString() is a reasonable "human" representation of the rational number but may have loss of accuracy.
The implementation uses the convenience BigRational.toBigDecimal() which uses the highest accuracy predefined math context in Java MathContext.DECIMAL128.
It would possibly make sense to reduce this accuracy but I would not go lower than the accuracy of Double, which is MathContext.DECIMAL64 .

Your proposed solution is similar to BigRational.toString() but using the very low accuracy of Float (which is equivalent to MathContext.DECIMAL32).

Maybe it makes sense to provide a BigRational.toString(MathContext) method which uses the argument to specify the desired accuracy of the string representation.

To put this another way:
BigRational x = BigRational.valueOf("8.804462619980757911125181749462772084351");
System.out.println(x.toFloat());
System.out.println(Float.valueOf(x.toBigDecimal().toString()));

Results in the output:
NaN
8.804462

Whereas:
BigRational x = BigRational.valueOf("8.80446261998075791112518174946277208435");
System.out.println(x.toFloat());
System.out.println(Float.valueOf(x.toBigDecimal().toString()));

Results in the output:
Infinity
8.804462

Whereas:
BigRational x = BigRational.valueOf("8.8044626199807579111251817494627720843");
System.out.println(x.toFloat());
System.out.println(Float.valueOf(x.toBigDecimal().toString()));

Results in the output:
8.804462
8.804462

The behaviour of x.toFloat() created an issue for me, I would have rather the behaviour had been more like Float.valueOf(x.toBigDecimal().toString()).

Ah sorry. I misunderstood the failure.

You are right.
The current implementations of toFloat() (and actually toDouble() are very naive.

The new implementation will probably go in this direction (still need to test some corner cases):

	public float toFloat() {
		return toBigDecimal().floatValue();
	}

Unit tests are looking good.

This is the new implementation including javadoc:

	/**
	 * Returns this rational number as a float value.
	 *
	 * <p>If the rational number cannot be represented as float then one of the following results will be returned:</p>
	 * <ul>
	 *   <li>&gt; <code>Float.MAX_VALUE</code> returns {@link Float#POSITIVE_INFINITY}</li>
	 *   <li>&lt; <code>-Float.MAX_VALUE</code> returns {@link Float#NEGATIVE_INFINITY}</li>
	 *   <li>&lt; <code>Float.MIN_VALUE</code> returns <code>+0.0f</code></li>
	 *   <li>&gt; <code>-Float.MIN_VALUE</code> returns <code>-0.0f</code></li>
	 * </ul>
	 *
	 * @return the float value
	 */
	public float toFloat() {
		return toBigDecimal().floatValue();
	}

and these are the unit tests:

	@Test
	public void testToFloat() {
		assertEquals(0.0f, valueOf(0).toFloat(), 0.0);
		assertEquals(123.0f, valueOf(123.0).toFloat(), 0.0);
		assertEquals(123.4f, valueOf(123.4).toFloat(), 0.0);
		assertEquals(-123.0f, valueOf(-123.0).toFloat(), 0.0);
		assertEquals(-123.4f, valueOf(-123.4).toFloat(), 0.0);
		assertEquals(0.33333333f, valueOf(1, 3).toFloat(), 0.0);
		assertEquals(0.6666667f, valueOf(2, 3).toFloat(), 0.0);

		assertEquals(8.804462f, valueOf("8.804462619980757911125181749462772084351").toFloat(), 0.0);
		assertEquals(8.804462f, valueOf("8.80446261998075791112518174946277208435").toFloat(), 0.0);

		assertEquals(5E-21f, valueOf(BigDecimal.ONE, new BigDecimal("2E20")).toFloat(), 0.0);
		assertEquals(0.0f, valueOf(BigDecimal.ONE, new BigDecimal("2E100")).toFloat(), 0.0); // underflow to +0.0f
		assertEquals(0.0f, valueOf(BigDecimal.ONE, new BigDecimal("2E9999")).toFloat(), 0.0); // underflow to +0.0f
		assertEquals(0, Float.compare(+0.0f, valueOf(BigDecimal.ONE, new BigDecimal("2E9999")).toFloat())); // underflow to +0.0f
		assertEquals(0, Float.compare(-0.0f, valueOf(BigDecimal.ONE, new BigDecimal("-2E9999")).toFloat())); // underflow to -0.0f

		assertEquals(2E20f, valueOf(new BigDecimal("2E20")).toFloat(), 0.0);
		assertEquals(Float.POSITIVE_INFINITY, valueOf(new BigDecimal("2E100")).toFloat(), 0.0); // overflow to +infinity
		assertEquals(-2E20f, valueOf(new BigDecimal("-2E20")).toFloat(), 0.0);
		assertEquals(Float.NEGATIVE_INFINITY, valueOf(new BigDecimal("-2E100")).toFloat(), 0.0); // overflow to -infinity
	}