Skip to content

parallel-letter-frequency: Needs major rework #1106

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

Closed
N-Parsons opened this issue Nov 14, 2017 · 2 comments
Closed

parallel-letter-frequency: Needs major rework #1106

N-Parsons opened this issue Nov 14, 2017 · 2 comments

Comments

@N-Parsons
Copy link
Contributor

N-Parsons commented Nov 14, 2017

As @AtelesPaniscus raised in #1099 (comment) there are a number of issues with the current implementation of parallel-letter-frequency.

These issues are:

  • It currently appears early in the config.json, meaning that in the current version of exercism.io, it's an early exercise.
  • The current tests don't check for parallel execution, and it's therefore possible to pass all tests with non-parallel code, and several submitted solutions are doing this.

I feel that the last point could be resolved by adding a hint about the use of multiprocessing and then trusting the learner to implement it faithfully (the new mentorship model will also help here). I think that will only work though, if we move the exercise to near the end of the list, so that people are confident enough to do it properly.

Having looked at the current implementation more closely, I think that there is a further issue in that it isn't truly using parallel execution, since it is using threading rather than multiprocessing. An article discussing parallelisation suggests that threading doesn't give simultaneous computation, but rather allows one thread to run while another is waiting for something (eg. I/O).

We do also have the option to forego an exercise if we don't think that it makes sense for this track.

I welcome opinions on how we proceed. My current suggestion would be to remove the exercise until we have resolved these issues and know what we want from this exercise.

@cmccandless
Copy link
Contributor

I am in favor of removing the exercise, perhaps permanently. The only way I can think of for verifying parallel execution is for the user to provide a thread-safe object/function and let the tests worry about process management.

@N-Parsons
Copy link
Contributor Author

I don't know that we necessarily need to be able to verify parallel execution (although if we could, we should), since we should be able to trust people to a certain extent - if they cheat, then they are cheating themself.

That being said, I've thought about it some more today and I feel like this exercise should probably be foregone. My reasoning is that Python doesn't have much of a focus on parallelism, particularly since the Global Interpreter Lock prevents parallelism within processes. In other languages, parallelisation is much simpler and is sometimes very much an integral part. If this changes in the future, or someone finds a good way to implement this exercise, we could then decide to "un-forego" it anyway.

I've created #1107 to mark the exercise as foregone and remove the exercise directory.

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

No branches or pull requests

2 participants