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

Update MAGEMin; add mpiexec dependency #4624

Closed
wants to merge 1 commit into from
Closed

Update MAGEMin; add mpiexec dependency #4624

wants to merge 1 commit into from

Conversation

boriskaus
Copy link
Contributor

this adds mpiexec as an executable product

this adds mpiexec as an executable product
@@ -45,6 +45,7 @@ platforms = expand_gfortran_versions(platforms)
products = [
LibraryProduct("libMAGEMin", :libMAGEMin)
ExecutableProduct("MAGEMin", :MAGEMin)
ExecutableProduct("mpiexec", :mpiexec)
Copy link
Member

Choose a reason for hiding this comment

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

I'm extremely confused: I don't see any mpiexec in the tarball, where it's coming from?

Copy link
Member

Choose a reason for hiding this comment

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

I think this is not failing because of a bug in the auditor: apparently we don't verify the products are in the final tarball. I believe we're accidentally picking up a file in the prefix from a dependency. This is completely wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It’s coming from the MPICH dependency but this may indeed not be the way to do thinks so feel free to reject

Copy link
Member

Choose a reason for hiding this comment

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

You may want to explain what you want to achieve 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to be able to call both mpiexec and MAGEMin as:

julia> using MAGEMin_jll
julia> run(`$(mpiexec()) -n 2 $(MAGEMin()) `)

Copy link
Member

Choose a reason for hiding this comment

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

julia> using MAGEMin_jll, MPICH_jll

julia> mpirun = addenv(mpiexec(), MAGEMin_jll.JLLWrappers.JLLWrappers.LIBPATH_env=>MAGEMin_jll.LIBPATH[]);

julia> run(`$(mpirun) -n 2 $(MAGEMin_jll.MAGEMin_path)`);
Running MAGEMin 1.0.1 on 2 cores {
‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾

VOL_SYS           +1.582647
RHO_SYS           +3253.910261
MASS_RES          +0.000010
Rank          : 0 
Point         : 0 
Temperature   : 1100.0000        [C] 
Pressure      : 12.00    [kbar]

SOLUTION: [G = -825.337] (37 iterations, 113.42 ms)
[-1011.909246,-1829.091670,-819.265707,-695.468292,-412.942264,-971.879545,-876.528123,-1073.651135,-276.626077,-1380.314700,]
 opx     0.23184 
 cpx     0.15210 
 spn     0.01395 
  ol     0.60211 
Point             0
__________________________________
MAGEMin comp time: +285.928000 ms }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, that's what I was looking for!
On Windows, we are not using MPICH_jll but MicrosoftMPI_jll instead. Do we need to do that with

if Sys.iswindows

else

end

, or is there a way to do this directly based on the mpiexec that is loaded in MAGEMin_jll?

Copy link
Member

Choose a reason for hiding this comment

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

No, you'll have to do something like

const mpirun = if Sys.iswindows()
    MAGEMin_jll.MicrosoftMPI_jll.mpiexec()
else
    MAGEMin_jll.MPICH_jll.mpiexec()
end

Ideally, at some point in the future we'll be able to use MPItrampoline for this, but I don't think we're there yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Actually, this should be more robust:

const mpirun = if MAGEMin_jll.MPICH_jll.is_available()
    MAGEMin_jll.MPICH_jll.mpiexec()
elseif MAGEMin_jll.MicrosoftMPI_jll.is_available()
    MAGEMin_jll.MicrosoftMPI_jll.mpiexec()
else
    nothing
end

You directly query the JLL packages whether they're available on the current platform, instead of hard-coding the platform-relations. In general I recommend using is_available() to make sure the artifacts are available in the current system.

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

Successfully merging this pull request may close these issues.

None yet

2 participants