Skip to content

Armstrong-numbers: add test case for 21897142587612075 #1487

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

Closed

Conversation

chvanikoff
Copy link

Most of solutions are using some sort of built-in "power" function, which can (ex.: Clojure, Erlang/Elixir, JS) rely on floating-point arithmetics under the hood (even though the interface functions can accept integers). The number 21897142587612075 is known to be armstrong one (http://mathworld.wolfram.com/NarcissisticNumber.html), but depending on the floating point arithmetic implementation used might cause false negatives on solutions that convert between numeric types, because 9^17 returns 16677181699666570 or 16677181699666568, but not 16677181699666569 which is expected.

@NobbZ
Copy link
Member

NobbZ commented Mar 25, 2019

I'd guess this number does not fit in uint32, and I'm not even sure if it would fit it's 64 bit counterpart.

Can current generators deal with such a value?

In theory, according to JSON this would even be rounded to float, as it exceeds the guaranteed 32 bit...

@NobbZ
Copy link
Member

NobbZ commented Mar 25, 2019

Addendum:

I'm pretty sure the Erlang generators would accept this value without further ado.

Golang might with small changes, but I have no clue about all the other generators.

@cmccandless
Copy link
Contributor

Python track does not currently have a generator in place (its in development), but Python language would have no trouble with this number. That said, I am fairly certain languages like Java and C++ would need to use a larger integer class.

@NobbZ
Copy link
Member

NobbZ commented Mar 25, 2019

I'm not only concerned about representability in a given language as some test from the spec is easily droppable, but much more if generators are able to read such a large number from a JSON without loss, as JSON does not know anything about integers or floats, it only supports numbers and allows the implementor to coerce as necessary.

I do know that the erlang library I use does parse numbers as erlang terms and due to the dynamic nature of the language, it will simply give me an int unless there is a dot . or an e in the number.

I know the JSON parser in the go lang standard library circumvents the issue by providing a special type json.Number that can be easily converted into a uint64, float or string (raw value). This type can even deal with numbers that are encoded as strings in the JSON. So we can use this type to circumvent the limitation both ways.

But I have no clue about other languages how they will be able to deal with such a large number in a JSON.

@coriolinus
Copy link
Member

I imagine that most track exercise generators are written in the track's language: if the generator can't handle this case, that's a good clue that the language probably wants to drop the case anyway.

Rust's serde_json provides the Number type, which can be f64, u64, or i64 depending on how you choose to convert it into a primitive. This is analogous to go's approach. Python is less explicit about what types it uses, but it appears to be able to decode an unsigned 64-bit number.

>>> big = 2**64 - 1
>>> big
18446744073709551615
>>> import json
>>> json.loads(json.dumps(big)) == big
True

I just don't see generator compatibility being a huge issue here.

@petertseng
Copy link
Member

I see that there is a discussion of numbers larger than 32 bits, as the number being discussed takes 55 bits.

I recommend that if there is any decision made on them here, deciders should consider whether the decision equally applies to the following:

  • grains (18446744073709551615, alternatively 2^64 - 1)
  • say (987654321123 and 1000000000000 both take 40 bits)

This comment should not be taken to mean either of "I {support, oppose} adding this test case to armstrong-numbers"

@ErikSchierboom
Copy link
Member

Can current generators deal with such a value?

I'm not sure, but in this case I would argue against adding this test case. Currently, armstrong-numbers is quite a basic exercise using only integers, but this would alter the exercise a bit and have it introduce a more complex type (floats/large integers). As a consequence, the exercise's difficulty would increase, which is something I'd like to prevent (we are already short of simple exercises).

@NobbZ
Copy link
Member

NobbZ commented Mar 29, 2019

As someone pointed out already the number does fit into 64 bit integers, which I do not consider extraordinary large, but instead common enough on modern platforms.

Except for JavaScript, I do not see any language that were unable to deal with those numbers by simply using the next bigger integer, or even taking the exercise as the introduction to sized integers (if it hasn't already happened before with grains).

And this test case is explicitely not introducing floats, it just shall avoid students to use floats in intermediate steps of the computation.

In erlang and elixir solutions are pretty common that use pow(n, m) to get n to the power of m. While this function takes integers or floats as input, it returns a float, always. Those students then used round/1 to get this number an integer again. With the current set of tests, this works as there are no rounding errors. So we needed a higher number where the conversion error actually changes the outcome of the test.

I'd be totally fine with finding a smaller number that fits into 32 bit as well. And to be honest, I do not even care if it produces false positives or negatives, as long as it produces a false result when using floating2integer conversion.

@rpottsoh
Copy link
Member

@ErikSchierboom I agree with your sentiment regarding integer size and introducing a larger class. Is there another exercise where a larger class is introduced? A quick scan of config.json only ever reveals integer and not a higher integer type. When is a good time to introduce a higher class of integer?

@ErikSchierboom
Copy link
Member

Is there another exercise where a larger class is introduced?

There are several: grains, largest-series-product, prime-factors.

@ErikSchierboom
Copy link
Member

When is a good time to introduce a higher class of integer?

This is of course, the million dollar question :)

@SleeplessByte
Copy link
Member

In JavaScript we explicitly provide big-integer.js for grains because of this.

@rpottsoh
Copy link
Member

There are several: grains

I guess I was expecting something different. Like in config.json indicating a topic other than integers, which isn't wrong but doesn't communicate that a higher class of integer is going to be covered/introduced. Perhaps that isn't the place to communicate that point. It is only obvious by reviewing the test suite that some new type that hasn't been covered before is being used... GrainsTest.cs is like this and so is the test suite I wrote for Delphi. I just expect the integer type to be used to be UInt64. I'm not sure where I am going with this... I'll just close by stating I wonder if it wouldn't be better to include a hint in the README or a comment in the test suite to introduce the new type. This would be track specific, perhaps just a comment in the canonical data to maintainers that it might be necessary to include a hint of some sort if this exercise will be introducing a new integer type....

@sshine
Copy link
Contributor

sshine commented Jun 26, 2019

@chvanikoff: This message is just to inform you that this commit somewhat depends on #1492.

@yawpitch
Copy link
Contributor

Putting this on hold per #1560

@yawpitch yawpitch added the hold label Oct 17, 2019
Base automatically changed from master to main January 27, 2021 15:31
@ErikSchierboom
Copy link
Member

So in this case, I'd argue that the added test case offers not much value over the existing test cases, with the downside being that track maintainers need to be careful implementing it due to possibly needing to change the involved data type. IMHO this goes against the goal of this exercise, which is to have a simple recursion/looping exercise.

If anyone disagrees with me, let me know. Otherwise, I'll be closing this PR in a couple of weeks.

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.

10 participants