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

powc of zero returns nan #114

Closed
domna opened this issue May 18, 2023 · 2 comments · Fixed by #116
Closed

powc of zero returns nan #114

domna opened this issue May 18, 2023 · 2 comments · Fixed by #116

Comments

@domna
Copy link
Contributor

domna commented May 18, 2023

When I execute the following line

Complex64::from(0.).powc(Complex64::from(1.))

I get nan as the output.

Shouldn't this be zero instead of nan?

@cuviper
Copy link
Member

cuviper commented May 18, 2023

Ouch, that's a big oversight. This probably comes from the implementation's r.ln() when r (the magnitude) is zero. We should probably add a special case for this... I'm not sure about how to handle the minutia of float +/-0 though. Maybe there's precedent we can draw from libraries in other languages.

@domna
Copy link
Contributor Author

domna commented May 19, 2023

Yes I was also suspecting the ln as well. It seems that C does just a check for zero and uses a similar formula: https://github.com/eblot/newlib/blob/2a63fa0fd26ffb6603f69d9e369e944fe449c246/newlib/libm/complex/cpowf.c#L47

I'm currently using a workaround for this:

trait ComplexPower {
    fn pc(self, exp: Complex64) -> Complex64;
}

impl ComplexPower for Complex64 {
    fn pc(self, exp: Complex64) -> Complex64 {
        if self.re == 0. && self.im == 0. {
            return Complex64::from(0.);
        }
        self.powc(exp)
    }
}

which does exactly this and it seems to be fine for me (but should rather use the abs of the complex number, though).
If I'm thinking correctly, I think for floats which are very small but not exactly zero it is fine as the ln will be still be properly defined but negative and very large (which we want here). Negative residuals will not occur as it uses the amplitude in the implementation.
I could prepare a PR for this.

bors bot added a commit that referenced this issue 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 <[email protected]>
Co-authored-by: Florian Dobener <[email protected]>
Co-authored-by: Josh Stone <[email protected]>
@bors bors bot closed this as completed in a78ab81 Aug 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants