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

broken nextpow functions #4819

Closed
stevengj opened this issue Nov 15, 2013 · 4 comments
Closed

broken nextpow functions #4819

stevengj opened this issue Nov 15, 2013 · 4 comments
Labels
bug Indicates an unexpected problem or unintended behavior needs decision A decision on this change is needed
Milestone

Comments

@stevengj
Copy link
Member

I've been looking at the prevpow/nextpow functions, and they seem somewhat broken to me:

  • nextpow2(n) != nextpow2(2, n): the former returns the power-of-2 value, and the latter returns the exponent. e.g. nextpow2(9) is 16, but nextpow(2, 9) is 4. Similarly for prevpow.
  • nextpow2(0) == prevpow2(0) == 0. I can only understand this behavior under the theory that zero is 2^–∞, but that seems questionable to me. nextpow(2, 0) and prevpow(2, 0) raise an InexactError (I would have expected a DomainError, maybe?).
  • prevpow(2, 1) correctly returns 0 (1 == 2^0, see first point), but nextpow(2, 1) throws a DomainError, which is certainly wrong.
  • nextpow(2, -7) and prevpow(2, -7) raise DomainErrors, but nextpow2(-7) gives -8 and prevpow2(-7) gives -4.
  • The documentation for these functions does not clarify any of the above behaviors.
@JeffBezanson
Copy link
Sponsor Member

@timholy can we change the meaning of nextpow(a, x)?

@timholy
Copy link
Sponsor Member

timholy commented Nov 15, 2013

My contribution was nextprod, I don't think I have any special investment in nextpow. Indeed, changing nextpow to return power-of-2 value will make it more consistent with nextprod.

(I get confused about those names myself sometimes.)

@JeffBezanson
Copy link
Sponsor Member

Good. I see the uses in Base look like a.^nextpow(a, x), so the change seems warranted. I checked the ~100 packages I have cloned and I don't see any uses so far.

@stevengj
Copy link
Member Author

(Probably for 0.3, though.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior needs decision A decision on this change is needed
Projects
None yet
Development

No branches or pull requests

3 participants