adamwhitehat / bigrational Goto Github PK

Description
numbers like -0.0001 have a whole part of 0 and a fractional part of 1 /10000, but whole and fractional are positive (sign = 1).
So nothing stores the sign of the complete number.

Error/Exception Message
If you try the following in RoslynPad:

#r "nuget: ExtendedNumerics.BigRational, 2023.162.546"

using ExtendedNumerics;
var value = new BigRational(0.000001);
value.Dump();
(-value).Dump();

you'll get:

Expected Behavior
(-value).Sign should be -1 and value.Sign should be 1
And value.Whole.Sign can remain 0, but then value.Fraction.Sign should become -1 in the negative case.
Actual Behavior
Both signs are 0 due to the sign being taken entirely from the whole part.

Notes
Explicitly creating a negative fraction results in Sign = 0

new BigRational(0, -1, 1000000).Dump();

Because the sign of the Fractional isn't even preserved when negating the entire number.

Description
There's an arithmetic bug with negative numbers, e.g. add -7 and -6 together and halve to find the mid point. I get -5.5, not the expected -6.5.

Steps To Reproduce

[TestMethod, TestCategory("Arithmetic")]
public void TestMidPoint()
{
	var low = (BigRational)(-7);
	var high = (BigRational)(-6);

	// Take the mid point of the above 2 numbers
	var mid = (low + high) / (BigRational)2;

	Assert.IsTrue(mid > low);
	Assert.IsTrue(mid < high);
}

Took me a while to realize I wasn't going crazy.

For example, if you divide -1 by -1 you get -1 (you should really get 1)

public static BigRational operator +(BigRational rational) => Abs(rational);

Thats's mathematically incorrect: "+ (-5)" equals "-5" not "5"

Should be:

public static BigRational operator +(BigRational rational) => rational;

The only version that was uploaded to NuGet is old.

Please upload the current version =D

Description
A clear and concise description of what the bug is.

[Test]
public void BigRationTest()
{
    var a = new BigRational(BigInteger.Parse("160000000000"));
    var b = new BigRational(BigInteger.Parse("249"), new Fraction(0.999));
    var c = BigRational.Divide(a, b);
    Console.WriteLine((decimal)c);
    Console.WriteLine(BigRational.Sqrt(c));
    // this result in 25348.917835671342685370741483M
    var bigRational = (decimal)BigRational.Sqrt(c);
    // this result in 25298.271877941387183714739055M, it is the same as the result from calculator from PC or phone
    var cc = DecimalMath.DecimalEx.Sqrt(160000000000m / 249.999m);
    // failed
    cc.Should().Be(bigRational);
}

Description
BigRational.NthRoot(new BigRational(50), 3);

returns exactly 3. I would expect that since this function takes a BigRational and not a BigInteger, that cutting off the decimal is a mistake? The FractionalPart shows a 0 Numerator and casting to any other floating point type is still an integer.

Error/Exception Message
Silent miscalculation

Expected Behavior
3.684031... or something indicating only whole numbers are supported.

Actual Behavior
3

Steps To Reproduce
Evaluate this
BigRational.NthRoot(new BigRational(50), 3));

Additional context/Comments/Notes/Rants
Thanks for creating this library. I hope I'm not missing something obvious :)

Description
BigDecimal Parse(Double input) => Parse(input.ToString(CultureInfo.CurrentCulture)); uses CurrentCulture to convert double into string, while next step uses BigDecimalNumberFormatInfo, which is InvariantCulture: public static BigDecimal Parse(String input) { return Parse(input, BigDecimalNumberFormatInfo); }

So, If CurrentCulture's NumberFormat is not similar to InvarantCulture, Parse + all releated tests will fail.
Example: new CultureInfo("ru-RU") has 'comma' for decimal point.

Same issue as BigDecimal/issues/16 reported by Degot. This issue exists in both these libraries.

In fraction.cs you have this constructor:

public Fraction(BigInteger numerator, BigInteger denominator)
{
	Numerator = new BigInteger(numerator.ToByteArray());
	Denominator = new BigInteger(denominator.ToByteArray());
}

I was comparing your implementation with the old Microsoft BigRational, and for my usage pattern the old (but buggy) Microsoft one was up to 10x faster, and the reason was the above constructor.

BigInteger is a struct, so it will be copied by value. If there's an issue that can be fixed by the above code then there is a big problem at play somewhere. I changed the code to the following and it still passes all your unit tests and at the same time speeds up my code 10 fold.

public Fraction(BigInteger numerator, BigInteger denominator)
{
	Numerator = numerator;
	Denominator = denominator;
}

Could you please enlighten me about this issue. Here's some code that I was using in my comparison test:

private readonly int _iterations = 1_000;
private readonly int _count = 1_000;

[TestMethod]
public void TestIteratingOnInsertionIndexDownDirectionAlternate2()
{
    List<ExtendedNumerics.BigRational> testSetLower = new List<ExtendedNumerics.BigRational>(_count);
    List<ExtendedNumerics.BigRational> testSetUpper = new List<ExtendedNumerics.BigRational>(_count);
    for (int i = 0; i < _count; ++i)
    {
        testSetLower.Add((ExtendedNumerics.BigRational)i);
        testSetUpper.Add((ExtendedNumerics.BigRational)(i + 1));
    }
    for (int iteration = 0; iteration < _iterations; ++iteration)
    {
        for (int i = 0; i < _count; ++i)
        {
            // Get pairs of number, take half way between them, and check it
            // is less than the high end and greater than the lower end
            ExtendedNumerics.BigRational newValue = (testSetLower[i] + testSetUpper[i]) / (ExtendedNumerics.BigRational)2;
            Assert.IsTrue(newValue > testSetLower[i]);
            Assert.IsTrue(newValue < testSetUpper[i]);
            testSetUpper[i] = newValue;
        }
    }
}