search

open-mpi / ompi

Open MPI main development repository
https://www.open-mpi.org
Other
2.07k stars 844 forks source link

Parameterize and move Fortran MPI bindings modulefiles install location #12649

Open jsquyres opened 3 days ago

jsquyres commented 3 days ago

See individual commit messages for details.

This PR is split into 2 commits so that we can cherry-pick one of them to the v5.0.x branch and still leave the default install location as $libdir.

Refs #12600

FYI @minrk

ggouaillardet commented 3 days ago

The first commit (e.g. add --with-mpi-moduledir option) looks good to me.

About the second one, well, you know what they say "if it ain't broke, don't fix it". It won't change anything for those who are already doing the right thing (e.g. have correctly configured MPI wrappers, and either use them or get the flags from mpifort --showme). But it might break stuff for those who did it the wrong way but somehow figured out how to get the job done. In all fairness, those who do not understand what they are doing, or use borked wrappers, or assume the Fortran module files are always in $PREFIX/include and sing kumbaya might get the illusion things are now working out of the box. In my not so humble opinion, that is not an upside for me (quite the opposite indeed), so regarding the second commit, I invite you to try reaching a consensus with other folks, look for a tie-breaker or simply disregard my opinion.

minrk commented 3 days ago

Thanks, both changes look great to me and I think are clearly the right choice by default.

jsquyres commented 3 days ago

I had the docs not quite correctly split across the 2 commits; I just fixed that and re-pushed.

@ggouaillardet makes some fair points:

But it might break stuff for those who did it the wrong way but somehow figured out how to get the job done.

I don't have too much heartbreak for breaking these use cases. It may even cause such cases to fix what they're doing wrong (i.e., switch to using a supported method).

In all fairness, those who do not understand what they are doing, or use borked wrappers, or assume the Fortran module files are always in $PREFIX/include and sing kumbaya might get the illusion things are now working out of the box.

I hear what you're saying. I think your 2 examples slightly contradict each other, though: the first one worries that we're going to break people who are doing the wrong thing (but who currently still manage to work); the second one worries that people doing the wrong thing are now going to work.

I guess my main thought is: if the Fortran world's conventions have changed to put modulefiles in $includedir, shouldn't we also change to do that? We're not trying to be Fortran trendsetters here -- we should be trying to do what the rest of the Fortran community does.

That being said, I don't have too strong of opinions here -- I think it would be ok to make this change at an Open MPI major release (i.e., 6.0), but if there's strong opposition to it, then I'll be happy enough with just adding --with-mpi-moduledir and not changing the default.

So we have @jsquyres and @ggouaillardet's opinions here -- anyone else want to chime in?

rhc54 commented 3 days ago

Just a suggestion: have you considered asking the other packagers on your mailing list? Probably a concern about breaking them without warning - I haven't seen anyone else from that community raising or commenting on this issue.