Bug in Baffling Birthdays Tests

The tests for Baffling Birthdays is not testing the whether the estimated probability is range. This:

function Test-Probability($Probability, $Expected, $Tolerance) {
    ($Probability -ge $Expected - $Tolerance) -or ($Probability -le $Expected + $Tolerance)
}

should be this:

function Test-Probability($Probability, $Expected, $Tolerance) {
    ($Probability -ge $Expected - $Tolerance) -and ($Probability -le $Expected + $Tolerance)
}

Also, in order the tests to complete in a reasonable amount of time, the tolerance needs to be higher than 1%. Through trial-and-error I found that 5% seems to work if the number of runs for testing random birthdays of the specified number of people is 1000.

I can do a PR to make this change if you’d like. I’ve never made a PR to an exercism repo before. Where can I find the documentation on what to do?

Actually a better fix for the tests would be this:

$tolerance = 5
function Test-Probability($Probability, $Expected, $Tolerance) {
    [Math]::Abs($Probability- $Tolerance) -le $Tolerance
}

Also, the example code has a bug. The return value from Invoke-BafflingBirthdays should be this:

$count * 100.0 / $runs

Can someone please tell me how to go about doing a PR for this?

What’s a reasonable amount of time? How did you get to 1000?

I’m able to run 5,000 tests for 73 people in bash in 2.5 seconds. With 5,000 simulations, a 2% tolerance should be plenty. At 1000 tests, my simulations suggest 4% ought to suffice.

Changing the or to an and is Powershell specific. The rest looks like it may belong in the upstream problem spec.

+cc @glaxxie

A reasonable amount of time is 1 to 2 seconds. On my PC, running Ubuntu 22.04 in WSL with Windows 11, all the tests completed in about 1.5 seconds when I used 1000. My main goal is to have the tests run reliably without timing out.

The or, and thing is Powershell-specific. I did a GitHub search on the exercism org to see what other tracks are using this exercise, and it looks like the only other one is C#. The example and tests looked right for that language.

Is this the right place to do that? If not, where?

This is the right place :slight_smile: We just need to wait for the maintainer to chime in.

1 Like

I tried using 5000 runs and 2% tolerance. It looks like it works, but it takes anything from 7 to 10 seconds. I’ll submit the solution and see if it runs fast enough on the server.

None the less, if the tests ought to be setting a fixed threshold, that threshold is something that might make sense to set independently of the track implementation/details.

1 Like

It’s not fast enough on the server.

I modified my code that finds the shared birthdays to be more efficient. Now my local run completes in about 3-4 seconds for 5000 runs and a 2% tolerance.

I pulled down the code from exercism/powershell and ran it locally. It is quite slow, even for just 1000 iterations. I took about 5 seconds. I had to set the tolerance to 4% to get the tests to pass.

1 Like