Skip to content
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

Make it an error to set multiple threads if NEST was built without thread support #2658

Merged
merged 3 commits into from
Apr 13, 2023

Conversation

heplesser
Copy link
Contributor

Until now, setting more than one thread if NEST was built without thread support resulted only in a warning. This led to a considerable number of tests being run in a meaningless way, and potentially causing problems in MPI-based test.

This PR makes it an error to try to set more that one local thread if NEST was built without thread support. The PR also ensures that all tests requiring multiple threads are skipped.

@heplesser heplesser added S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. I: Behavior changes Introduces changes that produce different results for some users labels Apr 10, 2023
@heplesser heplesser added this to PRs in progress in Kernel via automation Apr 10, 2023
@JanVogelsang
Copy link
Contributor

Apart from the formatting issues, the changes look good to me!

Copy link
Contributor

@med-ayssar med-ayssar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just randomly checking some interesting PRs.

@jougs, if you don't mind, I can review it instead of you :D

@heplesser: It looks good!

@heplesser
Copy link
Contributor Author

@med-ayssar Thanks!

@jougs Since it (minimally) changes behaviour (some scripts will fail now), I'd appreciate your input as well.

Copy link
Contributor

@jougs jougs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a very good change and am shocked that this was only a warning. Thanks!

@heplesser heplesser merged commit d5420fd into nest:master Apr 13, 2023
6 checks passed
Kernel automation moved this from PRs in progress to Done (PRs and issues) Apr 13, 2023
@heplesser heplesser deleted the make-bad-threads-error branch April 14, 2023 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: Behavior changes Introduces changes that produce different results for some users S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation.
Projects
Kernel
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants