Make fraction equality less strict

[brunoSnoww]

This PR fixes equality of fraction when there is loss of precision during conversions as addressed in #2918. It replaces the strict equality when dealing with fractions by the function nearlyEqual. It also changes the equals function inside the prototype of Fraction in order to make consistent with the other changes.

[josdejong]

Thanks Bruno, this PR looks solid.

I have some doubts about whether this approach is a good one. The change needed in the unit test simplifyAndCompare('3+sin(4)', '2.2431975046920716') triggered me. I think we have two different things going on here:

1. We want to handle round-off errors in floating point numbers smoothly. We do this by using nearlyEqual and an epsilon. That works well.
2. We want to silently convert a number into a Fraction only when the value can actually be represented as a fraction. We should not convert irrational numbers into Fractions, since that is not mathematically correct. It feels wrong to turn the result of simplify('3+sin(4)') into a Fraction since I know the result of sin(4) is an irrational number.

Thinking aloud here: if we want to keep (2) strict, we should see if we can solve the issue the other way around: instead of comparing a number and a Fraction by converting the number into a Fraction and then comparing two Fractions. Maybe we should turn the Fraction into a number and then compare the numbers using nearlyEqual. If we do that at the core of mathjs, it would mean changing the default of converting a number into a Fraction to converting a Fraction into a number, ie. number==fraction would change both sides to a number, but also number+fraction would change both sides into a number.

Another option would be to create a special case in the `equality functions to handle comparing numbers and Fractions differently, but I'm not sure we should go that way.

Alternatively, we could simply accept this limitation:

const math = create(all, { number: 'Fraction' })
math.createUnit('rotation', {definition: '360 deg', aliases: ['tour','tr']})

const a1 = math.unit(360, 'deg')                   // a1 has a value being a number
const a2 = math.unit(math.fraction(360), 'deg')    // a2 has a value being a Fraction
const a3 = math.fraction(math.unit(360, 'deg'))    // not supported right now but we could implement that
const b = math.unit(1, 'rotation')                 // b has a value being a Fraction

expect(b.equals(a1))                   // Error (cannot convert the internal value of a1 to a Fraction)
expect(b.equals(a2))                   // true
math.evaluate('360 deg == 1 rotation') // true

We could consider implementing support for math.fraction(unit) to make it easier to resolve these kind of conflicts.

I have to think this through a bit more 🤔 . Do you have any thoughts?

[brunoSnoww]

Thank Jos for the suggestions. Your point about not converting irrational numbers into fractions makes sense, that is not mathematically correct. Your suggestion of turning the Fraction into a number and then comparing the numbers using nearlyEqual seems like a possible solution, IMHO, but for now the most straightforward approach would be supporting math.fraction(unit) I guess. But overall I'd expect that math.unit(1, 'rotation') should be considered the same math.unit(360, 'deg'). Thanks again !

[josdejong]

But overall I'd expect that math.unit(1, 'rotation') should be considered the same math.unit(360, 'deg'). Thanks again !

Yes agree. I think there are two underlying issues here. The first is discussed in #2734: mathjs functions do not automatically convert number inputs into the configured numeric data types (like BigNumber or Fraction). And second: the built-in units are created with a number as value. We could think through changing their values to BigNumber and Fraction if that is configured. But that cannot be done in all cases: some units have an irrational value and cannot be turned into a Fraction, and for irrational numbers we need high precision values to represent them as a BigNumber with actual high precision. That will require some investigation and at this point I'm not sure if that is worth the effort and will result in a good solution or not.

For now I think we should keep things simple:

1. Close this PR: this approach unfortunately it will give unwanted side effects
2. Implement math.fraction(unit), to make solving these kind of issues easier. That requires little effort
3. We can add a section in the docs about Units to explain that the built-in units always have a number as value

Agree?

[brunoSnoww]

Agree. Let's close this PR and do the math.fraction(unit) solution🙃

[josdejong]

👍

@brunoSnoww I've created a PR implementing math.fraction(unit), can you maybe have a short look at that?