exercism / prolog

Exercism exercises in Prolog.
https://exercism.org/tracks/prolog
MIT License
29 stars 39 forks source link

Add exponent tests for complex-numbers #64

Open mbramson opened 6 years ago

mbramson commented 6 years ago

The complex-numbers exercise instructs users to implement the exponent function in the README, but there are no tests for this function and no function stub. As a result, no users have yet implemented this function.

I added 5 tests for the exponent function as well as an exponent stub function to hopefully better direct users to complete the exercise.

Some commentary on the process of coming up with this solution. I ran into a few cases where I got the warning "test succeeded with choicepoint". This sent me down a rabbit hole or two which might lead some who are on the second exercise to get confused. That said, I'm 3 days into prolog at this point, and haven't done any of the later exercises, and so am not sure how the complexity will scale as I get further in. That might be just fine, or that might indicate that this function was too difficult for this exercise, I lack the perspective to say at this point.

Average-user commented 6 years ago

test succeeded with choicepoint

That happens when you don't use the "!" operator, to end a search point.

qjd2413 commented 6 years ago

@Average-user is that something we could test for? I'm not sure if it's possible, but it would be useful to look into.

with regards to the actual content of the PR, the test logic isn't correct-- the first test with X \= 1 returns true because X = 1.0. Something about floating point arithmetic, probably.

Average-user commented 6 years ago

with regards to the actual content of the PR, the test logic isn't correct-- the first test with X \= 1 returns true because X = 1.0. Something about floating point arithmetic, probably.

You should use =\=:

?- 1.0 \= 1.
true.

?- 1.0 =\= 1.
false.
qjd2413 commented 6 years ago

Well I mean more that, with a correct exponent function, X should equal 1 (ignoring floating point), but these tests assert that X doesn't equal 1

Average-user commented 6 years ago

The complex-numbers exercise instructs users to implement the exponent function in the README, but there are no tests for this function and no function stub. As a result, no users have yet implemented this function.

@mbramson there are tests for the exponent function. Please follow them.

mbramson commented 6 years ago

@Average-user I didn't even know that https://github.com/exercism/problem-specifications/blob/master/exercises/complex-numbers/canonical-data.json was a thing! I just came up with those test cases myself. I'll update the PR with those test cases.

With regard to

with regards to the actual content of the PR, the test logic isn't correct-- the first test with X = 1 returns true because X = 1.0. Something about floating point arithmetic, probably.

You should use =\=:

?- 1.0 \= 1. true.

?- 1.0 =\= 1. false.

I'm not sure what you're saying I should do. The first case is likely the one we want right?

mbramson commented 6 years ago

@Average-user @qjd2413 Updated with new tests. Let me know what you think. Still up to change to =/= if you think that leads to a better test.

qjd2413 commented 6 years ago

Let's take the first test, for example, exponent((0,pi), (X,Y)), X \=-1, Y \= 0. This is the result of evaluating just exponent((0,pi), (X,Y)) image Because of floating point arithmetic, Y is not exactly 0, which is why these tests pass. I was incorrect about using == (because it doesn't represent unification), but we should be declaring equality-- \= and=\= declare inequality. Thus, a solution like

exponent((R,I), (Rr,Ir)) :-
  Rr is R + 1, Ir is I + 1.

passes your tests. I can't find any official documentation (or even unofficial) about the way to compare floats, but rationalize(X) =:= rationalize(<a>), rationalize(Y) =:= rationalize(<b>). seems to work for me.

mbramson commented 6 years ago

Oh wow. Well I feel stupid. I totally didn't realize that \= declared inequality.

mbramson commented 6 years ago

Okay, so the tests are actually testing things properly now, (although not exactly precisely, but up to three significant digits of precision). I don't love these tests (obviously infinitely better than before), but I couldn't find a good comparison operator that allowed for success when things were equal within some delta, so I basically wrote that myself.

As a result, these tests aren't as readable as they might be, but they are correct.

Let me know what you think @Average-user and @qjd2413

mbramson commented 6 years ago

As you pointed out =:= does not work here because of floating point arithmetic. I could use the exact floats that I get out of my solution, but that's assuming that everyone's solution will have the exact same amount of float driven error. I think that's a bad assumption as anyone else who encounters such a deviation will have a pretty bad debugging experience as a result.

qjd2413 commented 6 years ago

Could you create a floatEqual rule that looks something like

floatEqual(X, Y, Precision):- X < Y + Precision, X > Y + Precision.

so the tests are easy to understand on first glance?

ErikSchierboom commented 1 year ago

@mbramson Are you interested in getting this PR over the line?