-
Notifications
You must be signed in to change notification settings - Fork 10
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
Dependency Bump #66
Dependency Bump #66
Conversation
The MPI enabled LAMMPS_jll doesn't load on windows. This seems to be similar to JuliaSmoothOptimizers/MUMPS.jl#110. A temporary fix could be to disable MPI windows specifically. |
Disabling MPI on Windows is fine. |
test/runtests.jl
Outdated
@test LAMMPS.API.lammps_config_has_mpi_support() == 0 | ||
else | ||
@test LAMMPS.API.lammps_config_has_mpi_support() != 0 | ||
@test success(pipeline(`$(MPI.mpiexec()) -n 2 $(Base.julia_cmd()) mpitest.jl`, stderr=stderr, stdout=stdout)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should make this test error if it is run without MPI support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the plan is to fix MPI for windows and until that just have our tests automatically fail on windows?
I think we should also check wheter MPI is enabled in the init function and display a warning if it isn't. We should probably do the same for errors as well, as the REPL just crashes when lammps encounters an error if that isn't the case - Alltough starting from the 2024 versions errors are allways enabled
test/runtests.jl
Outdated
@test success(pipeline(`$(MPI.mpiexec()) -n 2 $(Base.julia_cmd()) mpitest.jl`, stderr=stderr, stdout=stdout)) | ||
end | ||
@test LAMMPS.API.lammps_config_has_mpi_support() != 0 | ||
@test success(pipeline(`$(MPI.mpiexec()) -n 2 $(Base.julia_cmd()) mpitest.jl`, stderr=stderr, stdout=stdout)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I meant, make the mpitest.jl
fail if it is run without MPI support. In my earlier hybris, I trusted this test to not work without MPI support, but it would just happily run with LAMMPS not using MPI.
The new mpitest fails on my windows machine, if I remove the if statement. So it should properly detect if MPI doesn't work |
No description provided.