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: don't use pow for integer power ufunc loops. #7496

Merged
merged 1 commit into from
Apr 2, 2016

Conversation

ewmoore
Copy link
Contributor

@ewmoore ewmoore commented Apr 1, 2016

Fixes gh-7405.

if (in2 & 1) {
out *= in1;
}
in2 >>= 1; /* Shift exponent down by 1 bit */
Copy link
Member

Choose a reason for hiding this comment

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

Please put the comment on the line before the code.

@matthew-brett
Copy link
Contributor

Could you put a test in maybe from the cited issue?

out *= in1;
}
in2 >>= 1; /* Shift exponent down by 1 bit */
if (in2 == 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Does this really speed things up at all? You are doing one check per iteration in order to save yourself a single multiplication in the last one, does it pay off.

If there is any performance to be gained there, you could alternatively peel off the first multiplication as an assignment, so that a single check is needed:

out = in2 & 1 ? in1 : 1;
in2 >>= 1;
while (in2 > 0) {
    in1 *= in1;
    if (in2 & 1) {
        out *= in1;
    }
    in2 >>= 1;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no idea really. This code is lightly adapted from the integer power code used for scalars.

@ewmoore
Copy link
Contributor Author

ewmoore commented Apr 1, 2016

Test added. I switched to using your version @jaimefrio. When I tested this, it either made no difference or a minuscule difference in run time with your version being the ever so slightly faster.

@ewmoore
Copy link
Contributor Author

ewmoore commented Apr 1, 2016

But in fairness, eliminating one multiplication is a bigger deal when finding squares or cubes relative to 62 powers.

@jaimefrio jaimefrio merged commit 6ab89b8 into numpy:master Apr 2, 2016
@jaimefrio
Copy link
Member

Thanks Eric!

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

Successfully merging this pull request may close these issues.

None yet

4 participants