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.

Sign up for GitHub

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

Jump to bottom

Abort NEST if run using mpirun but compiled without support for MPI #2533

Merged
merged 4 commits into from
Nov 29, 2022
Merged

Abort NEST if run using mpirun but compiled without support for MPI #2533

merged 4 commits into from
Nov 29, 2022

Conversation

jougs
Copy link
Contributor

@jougs jougs commented Nov 25, 2022

This fixes #2381.

@jougs jougs added T: Enhancement New functionality, model or documentation S: Normal Handle this with default priority I: Behavior changes Introduces changes that produce different results for some users labels Nov 25, 2022
@jougs jougs self-assigned this Nov 25, 2022
@jougs jougs added this to PRs in progress in Kernel via automation Nov 25, 2022
@jougs
Copy link
Contributor Author

jougs commented Nov 25, 2022

Should this be mentioned somewhere in the documentation, or do you consider this just an ordinary safeguard that goes without saying?

heplesser
heplesser reviewed Nov 27, 2022
View reviewed changes
Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

Since many will be using NEST via Slurm, I think it makes sense to mention srun as well. I don't think we need to add more information to the documentation, since this is a corner case and the error message very informative.

But why did the "Full" test fail? I restarted the test just in case it was a random system problem.

nestkernel/mpi_manager.cpp Outdated Show resolved Hide resolved
@jougs @heplesser
Update nestkernel/mpi_manager.cpp
527a749 
Co-authored-by: Hans Ekkehard Plesser <hans.ekkehard.plesser@nmbu.no>
@jougs
Copy link
Contributor Author

jougs commented Nov 27, 2022

But why did the "Full" test fail? I restarted the test just in case it was a random system problem.

I have no clue. It's the same failure as in https://github.com/nest/nest-simulator/actions/runs/3550596187/jobs/5968895331, so I don't think this is related to the code here.

@heplesser heplesser changed the title Abort NEST if run using mpirun, but compied without support for MPI Abort NEST if run using mpirun, but compiled without support for MPI Nov 28, 2022
@med-ayssar
Copy link
Contributor

As this new testing feature will not be tested by the CI, I think it is important to add an extra unittest for this case and expect it to fail.

@med-ayssar
Copy link
Contributor

I have tested it and it works perfectly!

@heplesser heplesser changed the title Abort NEST if run using mpirun, but compiled without support for MPI Abort NEST if run using mpirun but compiled without support for MPI Nov 29, 2022
@heplesser heplesser merged commit 045c7c6 into nest:master Nov 29, 2022
Kernel automation moved this from PRs in progress to Done (PRs and issues) Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@heplesser heplesser heplesser approved these changes

@med-ayssar med-ayssar med-ayssar approved these changes

Assignees

@jougs jougs

Labels
I: Behavior changes Introduces changes that produce different results for some users S: Normal Handle this with default priority T: Enhancement New functionality, model or documentation
Projects
Kernel
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Prevent NEST from running under mpirun-control if not built with MPI
3 participants
@jougs @med-ayssar @heplesser