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

Fixes nan value for powc of zero #116

Merged
merged 8 commits into from Aug 13, 2023
Merged

Fixes nan value for powc of zero #116

merged 8 commits into from Aug 13, 2023

Conversation

domna
Copy link
Contributor

@domna domna commented May 22, 2023

Fixes #114

@cuviper I just added a check for zero here as I suggested in #114.

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
@domna
Copy link
Contributor Author

domna commented Aug 9, 2023

Is there anything I can still do to get this merged?

src/lib.rs Outdated
@@ -1908,6 +1897,15 @@ pub(crate) mod test {
Complex::new(1.65826, -0.33502),
1e-5
));
let z = Complex::new(0.0, 0.0);
assert!(close(z.powc(b), z));
assert!(z.powc(Complex64::new(0., 0.)).is_nan());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this result should be one -- in fact the standard f64::powf(x, 0.0) == 1.0 for any base, even NaN! I'm going to fix both powf and powc here...

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not mathematically correct though, 0^0 is indeterminate.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the prior discussions in rust-num/num-traits#78 and rust-num/num-traits#79. It's a pragmatic choice, especially to be consistent with std.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That wikipedia article also says:

Another example is the expression 0^0. Whether this expression is left undefined, or is defined to equal 1, depends on the field of application and may vary between authors. For more, see the article Zero to the power of zero.

This is consistent with the standard library powf -- even for NaN!
@cuviper
Copy link
Member

cuviper commented Aug 11, 2023

Let me know what you think of that additional change for 0 exponents. I'll tweak the release date when we're ready.

@domna
Copy link
Contributor Author

domna commented Aug 12, 2023

Let me know what you think of that additional change for 0 exponents. I'll tweak the release date when we're ready.

I think it makes sense. So I'm fine with it.

@cuviper
Copy link
Member

cuviper commented Aug 13, 2023

bors r+

bors bot added a commit that referenced this pull request Aug 13, 2023
116: Fixes nan value for powc of zero r=cuviper a=domna

Fixes #114 

`@cuviper` I just added a check for zero here as I suggested in #114.

Co-authored-by: domna <florian.dobener@physik.hu-berlin.de>
Co-authored-by: Florian Dobener <github@schroedingerscat.org>
Co-authored-by: Josh Stone <cuviper@gmail.com>
RELEASES.md Outdated
@@ -1,9 +1,19 @@
# Release 0.4.4 (2023-08-13)

- [Fixes nan value for `powc` of zero][116]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- [Fixes nan value for `powc` of zero][116]
- [Fixes NaN value for `powc` of zero][116]

@cuviper
Copy link
Member

cuviper commented Aug 13, 2023

bors r-

@bors
Copy link
Contributor

bors bot commented Aug 13, 2023

Canceled.

@cuviper
Copy link
Member

cuviper commented Aug 13, 2023

bors r+

@bors
Copy link
Contributor

bors bot commented Aug 13, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit a78ab81 into rust-num:master Aug 13, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

powc of zero returns nan
3 participants