Suggestion: Add a large-limit test to "Sieve" to enforce correct algorithmic complexity

I notice many community solutions value concise implementations - sometimes at the expense of efficiency.

A case in point is “sieve” where some solutions - including the most starred one I see - actually implement “trial division” rather than Eratosthenes. Currently the largest unit test is for N=1000 - I suggest bumping that to 10 million to weed out some of these implementations. Eratosthenes still handles 10M in milliseconds while trial division will time out.

Here is a test for 10M - just validating the number returned for simplicity:

#[test]
#[ignore]
fn find_primes_up_to_10_million_tests_complexity() {
    let input = 10_000_000;
    let output = primes_up_to(input);
    
    // There are exactly 664,579 primes under 10,000,000.
    let expected_count = 664_579;
    
    assert_eq!(output.len(), expected_count, "The number of primes returned is incorrect.");
}

Hi there!

Python maintainer here (but answering, because the concern is cross-track).

It is excellent you are proposing a new test. But. :slightly_smiling_face:

The clincher here is that a test runner only gets ~20sec to spin up a container and test a solution (see test runner interface). So if the solution to this test extends the run-time past that mark, the test becomes one that can only be run locally, it won’t work on the site.

That might be why you are seeing the community solutions you are seeing. Anything that takes longer than ~20sec times out.

But it might also be that it can be implemented for Rust - just not every track. :smile: For that, you will have to check in with the Rust maintainer.

cc: @senekor

1 Like

Generally speaking, we typically test correctness and not implementation. One could argue that the Sieve exercises is explicitly about an implementation, though.

1 Like

Given practice exercises are ideally solvable multiple ways, I’d argue the Sieve exercise shouldn’t be about testing a specific implementation. The Square Root exercise for example suggests a couple of approaches without forcing a particular one. So maybe this exercise could be made more general.

Sieve specifically calls out an implementation, though. There are other prime number exercises that are more general.

1 Like

The test seems reasonable but I’m against it being added to the problem specs.

Sieve, I believe, predates modern Exercism (v3) since I vaguely recall solving it in v2. Mentoring was required then to complete the exercise so a human should catch the wrong algorithm. Since mentoring is optional in v3, we don’t have that safety net so we’re relying on a test runner timeout if the wrong algorithm is used. That’s flaky especially on compiled language tracks. You could get a false negative on a track that runs fast but uses trial division or a false positive on a track that uses the correct method but runs slow.

2 Likes

To me, the problem with tests purposefully triggering a runner timeout is: A runner timeout produces a generic failure, no indication of any wrong behaviour on the students side. That makes endless loops or algorithmic problems indistinguishable from infrastructure hickups or such. It’s bad user feedback, and should be avoided.

2 Likes