Update canonical-schema.json with "timeout" property

[martinfreedman]

Added non-required timeout (in milliseconds) property to labeledTest.properties to support tests with timeouts, such as for alphametics and palindrome-products exercises. This would support automatic generation/update of the tests rather than specific track manual modification of the relevant exercise's test file.

[wolf99]

I have no opinion on if this should be added. My review comment is purely on how to correctly add this if it is to be added

[SaschaMann]

How will that value be determined? The time it takes for a test to run will vary massively between languages, hardware, implementation.

It should also be added to the example in the README and documented there

[SleeplessByte]

Do we expect a timeout? As in, do we want students to write code that times out?

[martinfreedman]

@SleeplessByte For the test tooling that supports it, it is quite clear that this is a condition for a test to not time out and that a timeout is a test failure. The whole point is to make clear in the problem specification as to what is expected in the exercise as for the two already mentioned exercises. This is where it should be. Without this, the problem specification is inaccurate and/or misleading. Whether a specific track maintainer uses this property or not, they know it is part of the test constraints for a solution to pass.

[martinfreedman]

@SaschaMann example added to Readme

[ErikSchierboom]

I wonder if instead of this being property, it should instead be a scenario (see https://github.com/exercism/problem-specifications/blob/main/SCENARIOS.txt). That avoids any contention with how timeouts will be different between tracks, but you'd still be able to somehow mark a test case as being a performance-related test case (this is of course still contentious).

For the timeouts, I think the tests.toml file is a perfect place to define these timeouts.

[martinfreedman]

I am happy to change to @ErikSchierboom 's suggestion using scenariosand tests.toml. IMV the key point is that these are performance constrained tests and this needs to be captured as this level of the exercise specification.

Shall I create a new PR and then link back this and then close this one?

[SleeplessByte]

@martinfreedman normally we would try to get some consensus, but you, Erik, and I now agree, so that's a consensus of at least three. Perhaps you want to discuss first what the scenario would be?

I would like to see: scenario: timeout or ineffecient or high-time-complexity and then we use error as usual. All of those would allow you to automate this in the test generator.

[ErikSchierboom]

I am happy to change to @ErikSchierboom 's suggestion using scenariosand tests.toml. IMV the key point is that these are performance constrained tests and this needs to be captured as this level of the exercise specification.

Great!

I would like to see: scenario: timeout or ineffecient or high-time-complexity and then we use error as usual. All of those would allow you to automate this in the test generator.

Yeah, something like that would be nice. Another option for a scenario is performance. 🤷

[martinfreedman]

I think "performance" scenario best captures what is being tested, whereas "timeout" states how to do that and can be a function of different languages and test frameworks. "Inefficient" and "high-time-complexity" also don't quite get to the fundamental point, that this is a performance test.

[SleeplessByte]

I would be happy to accept "performance".

[ErikSchierboom]

Works for me too!

[SaschaMann]

I don't like performance for this. performance suggests to me that it could also be about optimising the implementation in the language to make it faster without changing the algorithm, whereas the exercises you mention are primarily about algorithmic optimisations. The former would require significantly more track-specific changes, so I think the distinction is important.

(the change request is about the previous comments, not the name of the scenario)

[ErikSchierboom]

(the change request is about the previous comments, not the name of the scenario)

Eh, are you against or in favor of using performance as the scenario?

[SaschaMann]

Eh, are you against or in favor of using performance as the scenario?

Against it and wanted to explain why because the reason wasn't brought up yet, but if people disagree with that reason, it's fine by me, too.

Just wanted to clarify what the blocking change request is about.

[ErikSchierboom]

Do you have an alternative name perhaps?

[SaschaMann]

Maybe something like algorithmic-optimisation or algorithmic-optimisation-needed?

[martinfreedman]

I don't like performance for this. performance suggests to me that it could also be about optimising the implementation in the language to make it faster without changing the algorithm, whereas the exercises you mention are primarily about algorithmic optimisations. The former would require significantly more track-specific changes, so I think the distinction is important.

I disagree. Performance covers both and that is why I like it. Plus different languages might have different facilities. The exercises are both about combinatorics and that is likely to be the main use case for any exercise for these performance tests. Having built-in fast or optimised combinatorics alone will not or should not help.

[SaschaMann]

Performance covers both and that is why I like it.

That's precisely why I think it's unsuitable. Both cases require different handling in the implementation, which is why the broad term is too general for a scenario.

[martinfreedman]

I do not follow this since, both scenarios (if there are really two , not sure) require same handling in a test, in the case of F# & C# by using the xunit timeout attribute but still keeping the same test name.

[ErikSchierboom]

@exercism/reviewers Your thoughts on what the scenario should be named?

[SaschaMann]

I do not follow this since, both scenarios (if there are really two , not sure) require same handling in a test, in the case of F# & C# by using the xunit timeout attribute but still keeping the same test name.

They might require the same handling in a test but not necessarily in the changes to the instructions. Using different scenarios adds that additional information so that human maintainers can make use of it, even if auto generators handle them identically.

[BethanyG]

I am somewhat ambivalent to the name, as long as it gives me a clean way to flag tests that could be problematic, rather than turning off the runner altogether for an exercise. I favor any method that will also make it possible for me to add in explanation/warning for students. Something along the lines of:

"While you can use a naive or 'brute force' methods to solve this exercise, large inputs may cause processor or memory issues. The following tests are designed with large inputs and may timeout, fail, or cause problems if your code is inefficient."

or

"This exercise can be solved using recursion, but carefully check your code. Python does not have tail-call optimization, and larger inputs could cause problems. The following tests are designed with large inputs and may timeout, fail, or cause other issues if your solution is inefficient."

FWIW, Pytest (what we're using more or less for the Python test runner) has a slow flag, intended to be used for "resource-heavy" tests. That's probably what I'd map any of these terms to once they're in place.

To me, performance implies hitting a certain speed or processor or memory benchmark. Then again, with our test runner timeouts, we are indeed implying that there is one.

[ErikSchierboom]

I quite like "slow"!

[ErikSchierboom]

What do other people think of "slow" as the scenario name?

[martinfreedman]

Given the point that @SaschaMann made namely " whereas the exercises you mention are primarily about algorithmic optimisations." how about Optimise?

[SaschaMann]

At the risk of continuing to bikeshed: I don't like slow for the use cases outlined in this PR. Not all test cases that might require timeout handling of some sort are inherently slow. There are cases where a naïve implementation might be slow yet a solution that implements a more efficient algorithm won't be. In that case, tagging the case as slow doesn't really make sense to me but that's what we'd be doing in the canonical data.

@martinfreedman you've also downvoted the suggestion, could you elaborate why?

[ErikSchierboom]

Seeing that @SaschaMann and @martinfreedman have downvoted "slow", what do people things about the suggested "optimise"?

[kotp]

I like optimized, but it leaves the question unanswered for "optimized for what? Time, Memory, Energy, Readability?"

[martinfreedman]

I like what @BethanyG implies "Efficient" better than "Optimise" (and especially not "Optimize"). Or maybe "TimeEfficiency" as that is the specific issue I am addressing and offering a solution for.

In addition the expected BigO restriction could be added into a test name e.g. "Large Inputs require at least Logarithmic Performance" ) or (Linear or LogLinear etc.)

[ErikSchierboom]

Maybe time-optimization or optimize-time?

[martinfreedman]

@ErikSchierboom you mean "time-optimise" or "optimise-time". Most of us are not from the USA, why should we have the imposition of USA spelling?

[ErikSchierboom]

Most of us are not from the USA, why should we have the imposition of USA spelling?

Because that's what our style guide defines :)

[martinfreedman]

@ErikSchierboom OK, Then I like "optimize-time"

[ErikSchierboom]

Cool! @SaschaMann @kotp @BethanyG @SleeplessByte @wolf99: what do you think about optimize-time?

[SaschaMann]

I would prefer if it specified that this is about time complexity, not just time to run in general, but I'd be happy with either.

[ErikSchierboom]

I'd be perfectly fine with time-complexity or some variant.

[kotp]

On 22/01/14 07:17AM, Erik Schierboom wrote: I'd be perfectly fine with `time-complexity` or some variant.

optimize-time is not as good as optimize-for-time in my opinion. `time-complexity` sounds like Big O notation is implied, at least to me. But going in the right direction, I think.

[BethanyG]

I'm fine with time-complexity, optimize-time/time-optimize, or time-efficiency. 😄

[SaschaMann]

I'd be perfectly fine with time-complexity or some variant.

I was thinking optimise-time-complexity, I like optimise-time but on its own it has the same issues that I described above where it's not clear what one needs to optimise.

[ErikSchierboom]

Okay, anyone against using optimise-time-complexity then? :)

[ErikSchierboom]

Okay, one final chance. Does anyone object to using optimise-time-complexity for the name of the scenario?

[ErikSchierboom]

Okay, let's use optimise-time-complexity then. @martinfreedman could you update the PR (it also needs rebasing)?

[IsaacG]

@martinfreedman Are you still interested in working on this?