Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: BigDecimal.Pow(5d, 0.5d) no longer works #45

Open
MileyHollenberg opened this issue May 6, 2024 · 3 comments · May be fixed by nissl-lab/npoi#1350
Open

BUG: BigDecimal.Pow(5d, 0.5d) no longer works #45

MileyHollenberg opened this issue May 6, 2024 · 3 comments · May be fixed by nissl-lab/npoi#1350
Assignees
Labels

Comments

@MileyHollenberg
Copy link

Description
In the Readme there's an example which uses BigDecimal.Pow(5d, 0.5d) but this is no longer working as the Pow method no longer accepts a double value, only a BigInteger.

Error/Exception Message
Compile time error

Expected Behavior
Having the ability to raise a BigDecimal value to the power of a decimal based value

Actual Behavior
You can only raise BigDecimal's to the power of a integer value

Steps To Reproduce
Simply add BigDecimal.Pow(5d, 0.5d) somewhere in your code and see that it won't compile

Screenshots
N/A

Additional context/Comments/Notes/Rants
Seems it got removed in this commit

@AdamWhiteHat
Copy link
Owner

Sorry about that. I certainly didn't intent to remove that method. I review every file and line before I commit changes, I must have not been paying very close attention.

Thank you for bringing this to my attention.

I have build and released a new NuGet package, version 2025.1001.2.129 that contains this fix.

@MileyHollenberg
Copy link
Author

MileyHollenberg commented May 8, 2024

Thanks, this looks to be back again :). Would it maybe be possible to also get a version that accepts a BigDecimal as the first argument? I've tried to create one myself but this seems to be quite difficult (the LogN and Exp functions frequently freeze up due to how much work they would need to do)

For clarity, I was trying to get it to work like this (came from ChatGPT as I'm not the strongest with this kind of math in general so it might be flawed from the start already)

  public static BigDecimal Pow(BigDecimal @base, BigDecimal exponent)
  {
      // Convert exponent to natural logarithm
      BigDecimal logarithm = BigDecimal.Log(@base);
      
      // Multiply logarithm by the decimal exponent
      BigDecimal resultLog = BigDecimal.Multiply(logarithm, exponent);
      
      // Convert back to original scale
      BigDecimal result = BigDecimal.Exp(resultLog);
      
      return result;
  }

Bykiev added a commit to Bykiev/npoi that referenced this issue May 16, 2024
@Bykiev Bykiev linked a pull request May 16, 2024 that will close this issue
@AdamWhiteHat
Copy link
Owner

AdamWhiteHat commented May 18, 2024

Would it maybe be possible to also get a version that accepts a BigDecimal as the first argument?

For clarity, I was trying to get it to work like this (came from ChatGPT as I'm not the strongest with this kind of math in general so it might be flawed from the start already)

  public static BigDecimal Pow(BigDecimal @base, BigDecimal exponent)
  {
     ...

This method is essentially correct. The only thing it got wrong is there is no BigDecimal.Log() method; Use BigDecimal.Ln() instead.

I didn't think it was correct at first, so I implemented it using the same principal, then compared the methods. This is what I made:

public static BigDecimal Pow(BigDecimal @base, BigDecimal exponent, int precision)
{
	//  b^p = x^(p * log_x(b)) = e^(p*ln(b))
	// because log_x(b^p) = p * log_x(b)

	BigDecimal ln_b = BigDecimal.Ln(@base, precision);
	return BigDecimal.Exp(exponent * ln_b, precision);
}

But you are correct about behavior of LogN and Exp: Because these are implemented using the Taylor series, the asymptotic complexity depends almost entirely on its input; some values take hundreds of iterations to converge, others might take just 3.

And in the above code, we are calling both Ln and Exp, increasing the chances that one of them will not like its input value and bog down.

So for this reason, it would probably be better if I just wrote an exponentiation function directly. I have made a start on this. I don't anticipate running into any difficulties.

In the interim, use the above method for now, I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants