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

rational-numbers: Clarify that denominator can be 0 #1856

Merged
merged 2 commits into from
Oct 26, 2021
Merged

Conversation

SaschaMann
Copy link
Contributor

In some languages the denominator can be 0 and therefore the track's implementation of this exercise may expect it. Stating b != 0 in the description may be misleading if the track expects it.

In some languages the denominator can be 0 and therefore the track's implementation of this exercise may expect it. Stating `b != 0` in the description may be misleading if the track expects it.
@angelikatyborska
Copy link
Contributor

angelikatyborska commented Oct 13, 2021

When the denominator is 0, that is no longer a rational number, according to the definition (https://en.wikipedia.org/wiki/Rational_number). I would argue all tracks should implement the exercise in a way that 0 is not allowed.

@SaschaMann
Copy link
Contributor Author

In some languages used for numerical maths, 0 as denominator is allowed and behaves similar to Inf in floats.

The track shouldn't diverge from that in a reimplementation exercise like this, imo.

@wolf99
Copy link
Contributor

wolf99 commented Oct 13, 2021

If I can restate what Angelika has said, and that I agree with:

While some languages may indeed be generally able to handle fractions with a denominator that is 0, the result (Inf or otherwise) would not be a rational number. This line is specifically defining rational numbers, not fractions in general, and thus this change is not correct.

This is clear in the first line of the Wikipedia article on rational numbers, which is not related to any programming language at all, only mathematics: https://en.wikipedia.org/wiki/Rational_number

@SaschaMann
Copy link
Contributor Author

SaschaMann commented Oct 14, 2021

I don't think the mathematical definition is the only relevant consideration. After all, we also limit solutions to rational numbers representable by integers, and don't expect solutions to use arbitrary precision types. Same goes for the complex number exercise where we restrict ourselves to the limitations of floats and ints, even if the mathematical definition would allow all real numbers as real/imaginary components.

What if we add an admonition along the lines of

Note that mathematically, the denominator can't be zero. However in many implementations of rational numbers, you will find that the denominator is allowed to be zero with behaviour similar to +/-Inf in floating point numbers. In those cases, the denominator and numerator still can't both be zero at once.

Students read the first line and then might be confused that a denominator of 0 is actually allowed and expected/tested for. This has already occurred a few times. Having a hint on top will, in my expectation anyway, help prevent that.

@wolf99
Copy link
Contributor

wolf99 commented Oct 14, 2021

I see two sides

On one side, this seems innocuous enough.

On the other hand, this could be a little confusing for languages where this is not the case.
My understanding is that
specifications should aim to be the lowest common denominator, i.e that it can sets expectations for behavior across the widest set of languages possible. If languages wish to deviate from that for things that are specific to that language, that's fine but that language-specificness should not be upstreamed into the canonical spec.

This is my thoughts only. On the whole I don't think it matters too much as long as:

  1. It is clearly explained, which the latest proposal appears to be.
  2. This doesn't then lead to language-specific aspects being upstreamed into specs in general.

@angelikatyborska
Copy link
Contributor

I don't think the mathematical definition is the only relevant consideration. After all, we also limit solutions to rational numbers representable by integers, and don't expect solutions to use arbitrary precision types.

In this specific case (rational numbers) it is in the mathematical definition that they are represented by integers only.

with behaviour similar to +/-Inf in floating point numbers

This is language-specific IMO, Elixir has no notion of infinity and you cannot divide by the floating point 0.0 either.

I'm still convinced that what you're trying to add @SaschaMann would better fit in the instructions.append.md file in the Julia track than here.

@SaschaMann
Copy link
Contributor Author

SaschaMann commented Oct 15, 2021

In this specific case (rational numbers) it is in the mathematical definition that they are represented by integers only.

I should have been more clear: I meant integers as in 32 bit or 64 bit integers, not as synonym for whole numbers/the mathematical definition of integers. In other words, we don't expect solutions or track implementations to cover 68334988235624235424945359362330475855319106846720 / 35089875053457002743422165688479937777317917491200 even if mathematically that's a rational number.

This is language-specific IMO, Elixir has no notion of infinity and you cannot divide by the floating point 0.0 either.

The IEEE 754 standard for floating point arithmetic requires it, and at least in my experience when floating point numbers and arithmetics are taught or introduced, e.g. in school, uni, or textbooks, people generally talk about floating point numbers in the context of that standard. Since the instructions shouldn't be language-specific, I think it's fine if the general hint refers to that standard just like in those other learning contexts rather than any language's take on floats.

If you think this is confusing in the context of tracks that deviate from that standard, I'm happy to rephrase it, I just found it to be a simple comparison to a general understanding of floats.

This is overly verbose and touches upon the ordering which is not (yet, #1736) part of the spec, so I'd prefer my earlier suggestion, but what about:

Note that mathematically, the denominator can't be zero.
However in implementations of rational numbers, you may find that the denominator is allowed to be zero and behaves like an infinity-value where `-1/0` is smaller than all negative rational numbers, and `1/0` is larger than all positive rational numbers.
In those cases, the denominator and numerator typically still can't both be zero at once.

I'm still convinced that what you're trying to add @SaschaMann would better fit in the instructions.append.md file in the Julia track than here.

If I could insert the append anywhere in the file, that would be fine. Putting it at the end doesn't fix the issue however, since the instructions invite people to work on them 1 by 1 from the top, and I think the clarification should be done in the same context of where it's stated that they can't have a zero denominator mathematically.


My understanding is that specifications should aim to be the lowest common denominator, i.e that it can sets expectations for behavior across the widest set of languages possible. If languages wish to deviate from that for things that are specific to that language, that's fine but that language-specificness should not be upstreamed into the canonical spec.

I fully agree, but I think the current instruction is not the lowest common denominator but rather favours one particular take on it. I don't have any opinions on whether or not the implementation should allow division by zero, since in "real-world" implementations of rational numbers both views exist, but the instructions should allow for both without favouring one over the other.


This doesn't then lead to language-specific aspects being upstreamed into specs in general.

This probably deserves its own issue or PR, but since the current spec doesn't have tests on either option, it might be useful to add two optional/"scenario'd" test cases that either expect an error or don't expect one, to make it more obvious for maintainers that they should decide which one fits their language best.

@SaschaMann SaschaMann linked an issue Oct 19, 2021 that may be closed by this pull request
@ErikSchierboom
Copy link
Member

ErikSchierboom commented Oct 26, 2021

@angelikatyborska @wolf99 What do you think about the latest approach taken bij Sascha, where the information is presented in a note?

@wolf99
Copy link
Contributor

wolf99 commented Oct 26, 2021

The text seems clear and accurate to me.

Formatting as a fenced code block seems odd though. Is this normal usage in exercism markdown files?
If yes, then that is convention and I am ok with it. If not then not.

@SaschaMann
Copy link
Contributor Author

Formatting as a fenced code block seems odd though. Is this normal usage in exercism markdown files?

See https://exercism.org/docs/building/markdown/markdown (CTRL F "admonition")

@wolf99 wolf99 merged commit 3d713dc into main Oct 26, 2021
@wolf99 wolf99 deleted the SaschaMann-patch-1 branch October 26, 2021 14:44
@wolf99
Copy link
Contributor

wolf99 commented Oct 26, 2021

Well done @SaschaMann !

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

Successfully merging this pull request may close these issues.

5 participants