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

ndigits(big(0), 1) does not raise an error #16766

Closed
yuyichao opened this issue Jun 4, 2016 · 9 comments
Closed

ndigits(big(0), 1) does not raise an error #16766

yuyichao opened this issue Jun 4, 2016 · 9 comments
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@yuyichao
Copy link
Contributor

yuyichao commented Jun 4, 2016

I'm not sure if it should but it is in the test and I've just triggered it once while stress testing #13995 .

Either way, the test needs to be fixed with explicit test for this case added. The implementation possibly needs fixing too if an error is desired.

@Keno
Copy link
Member

Keno commented Jun 4, 2016

Also ndigits(1, 1) hangs for me.

@Keno Keno added the bug Indicates an unexpected problem or unintended behavior label Jun 4, 2016
@dpsanders
Copy link
Contributor

Shouldn't bases -1, 0 and 1 all give errors?

@Keno
Copy link
Member

Keno commented Jun 5, 2016

There may be an argument to be made that ndigits(x, 1) = x. I believe 0 and negative numbers are already errors.

@yuyichao
Copy link
Contributor Author

yuyichao commented Jun 5, 2016

iiuc base 1 can really only represent 0 so returning 1 in that case (ndigits(0, 1)) makes some sense (I'd be quite surprised if anyone really relies on that behavior though) all other cases should be error, which is what the failing test is testing (at least for BigInt).

@rfourquet
Copy link
Member

cf. https://en.wikipedia.org/wiki/Non-standard_positional_numeral_systems for an idea why ndigits(x, 1) = x can make sense, but it may be too non-standard to be implemented in base. I think I would also favor erroring on bases -1, 1, and 0, even when x==0.

@TotalVerb
Copy link
Contributor

We would need to have a bijective_ndigits for that to make sense, and I think that's outside the scope of Base.

rfourquet added a commit to rfourquet/julia that referenced this issue Jun 9, 2016
rfourquet added a commit to rfourquet/julia that referenced this issue Jun 9, 2016
rfourquet added a commit to rfourquet/julia that referenced this issue Jun 9, 2016
rfourquet added a commit to rfourquet/julia that referenced this issue Jun 17, 2016
JeffBezanson added a commit that referenced this issue Aug 10, 2016
See #16766. `ndigits(big(0),1)` doesn't raise an error, but we were
sometimes (randomly, rarely) testing that it does. This change makes
the test reliable. The issue of how this case should actually
behave is still open.
tkelman pushed a commit that referenced this issue Aug 11, 2016
See #16766. `ndigits(big(0),1)` doesn't raise an error, but we were
sometimes (randomly, rarely) testing that it does. This change makes
the test reliable. The issue of how this case should actually
behave is still open.

(cherry picked from commit dd09d19)
ref #17952
tkelman pushed a commit to tkelman/julia that referenced this issue Aug 16, 2016
See JuliaLang#16766. `ndigits(big(0),1)` doesn't raise an error, but we were
sometimes (randomly, rarely) testing that it does. This change makes
the test reliable. The issue of how this case should actually
behave is still open.
rfourquet added a commit to rfourquet/julia that referenced this issue Aug 17, 2016
rfourquet added a commit to rfourquet/julia that referenced this issue Aug 19, 2016
rfourquet added a commit to rfourquet/julia that referenced this issue Aug 26, 2016
mfasi pushed a commit to mfasi/julia that referenced this issue Sep 5, 2016
See JuliaLang#16766. `ndigits(big(0),1)` doesn't raise an error, but we were
sometimes (randomly, rarely) testing that it does. This change makes
the test reliable. The issue of how this case should actually
behave is still open.
@simonbyrne
Copy link
Contributor

What does a negative base mean?

@StefanKarpinski
Copy link
Member

Negative bases are a real thing, e.g. negabinary. Since I believe there's only one thing they can mean, we might want to just support them correctly. But clearly it's not a high priority.

@simonbyrne
Copy link
Contributor

Okay, but that isn't documented anywhere, nor are they supported by the base function.

rfourquet added a commit to rfourquet/julia that referenced this issue May 3, 2017
rfourquet added a commit that referenced this issue May 12, 2017
ndigits: check for invalid bases (fix #16766)
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
Projects
None yet
Development

No branches or pull requests

7 participants