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

Compilers unrecognised on Windows #9

Open
TysonRayJones opened this issue Feb 2, 2024 · 6 comments
Open

Compilers unrecognised on Windows #9

TysonRayJones opened this issue Feb 2, 2024 · 6 comments

Comments

@TysonRayJones
Copy link

Hi there,

Thanks very much for a wonderfully helpful Github Action!

I was wondering what the compiler command is when on Windows. I've tried

mpicc
mpic++
mpicxx

but none of them are recognised, though they work on ubuntu-latest and macos-latest.

My workflow is a very simple:

jobs:

  cpp_mpi:
    name: C++ (MPI)
    runs-on: windows-latest

    steps:
      - uses: actions/checkout@v4
      - uses: ilammy/msvc-dev-cmd@v1
      - uses: mpi4py/setup-mpi@v1

      - name: compile backend
        run: |
          mpicxx -c mycode.cpp

but my actions report that mpicxx is not recognised.

I can see in the CI that these commands should work, at least when using the Intel MPI compiler.

What am I doing wrong? Do I need to give the action extra clues about my os?

@TysonRayJones
Copy link
Author

Here are some logs reporting MS-MPI is installing fine, but msmpi (and mpicc, mpicxx, etc) being subsequently unrecognised

image

@TysonRayJones
Copy link
Author

I notice the tests don't even ever run the MS-MPI compiler... Why is it the default MPI compiler on Windows if it's unusable? 😕

TysonRayJones added a commit to TysonRayJones/InteroperableComplex that referenced this issue Feb 2, 2024
because MS-MPI seems bugged?
mpi4py/setup-mpi#9
@dalcinl
Copy link
Member

dalcinl commented Feb 3, 2024

Microsoft MPI does not have/use compiler wrappers. You should use environment variables to manually pass include-dir/library-dir flags and msmpi.lib, the list of vars is here: https://github.com/mpi4py/setup-mpi/blob/master/setup-msmpi.ps1#L39.

Please note that Windows is not an OS I have ultimate expertise with, therefore I cannot assert that things are properly setup for this action to be really useful. My primary motivation was to ease the testing of mpi4py. Anyway, mpi4py is able to build with CMake just fine with both Intel MPI and MSMPI, so that means things are kind of OK.

You should probably stay away from MSMPI and target Intel MPI instead. Intel MPI do have compiler wrappers (mpicc.bat, etc) and they support a newer version of the MPI standard.

@TysonRayJones
Copy link
Author

Ahh I see, that makes sense - and is another reason why Windows development is absolutely cursed 🙏
That's very helpful, thanks very much!

I definitely then think the Intel compiler should be the default. Otherwise, an attemptedly os-agnostic workflow which uses mpicc (as per the main README) will fail if trying to put os in a matrix, whereas defaulting Intel would enable it to work seamlessly.

@dalcinl
Copy link
Member

dalcinl commented Feb 4, 2024

I definitely then think the Intel compiler should be the default.

You are definitely right.

However, I'm quite conscious about backward compatibility. I cannot make intelmpi the default in a minor update, that should wait until at least release v2 of this action. And making a new major release just to change a default seems a little bit too much.

Otherwise, an attemptedly os-agnostic workflow which uses mpicc (as per the main README) will fail if trying to put os in a matrix, whereas defaulting Intel would enable it to work seamlessly.

Most os-agnostic attempts fail when Windows is added to a matrix!

Maybe we could map mpich -> intelmpi on Windows? After all Intel MPI is an MPICH derivative which is ABI compatible on Linux. That way an mpich value would work seamlessly in Linux, macOS, Windows.

@TysonRayJones
Copy link
Author

However, I'm quite conscious about backward compatibility. I cannot make intelmpi the default in a minor update, that should wait until at least release v2 of this action. And making a new major release just to change a default seems a little bit too much.

Ahh yep that's a fair consideration. Indeed I was going to suggest changing the default on the next version, but I suppose you're right it's an insufficient update (although I don't know what's standard r.e. github actions)

Most os-agnostic attempts fail when Windows is added to a matrix!

Hehe that's true, though I suppose one shouldn't widen the changes needed ¯\(ツ)

Maybe we could map mpich -> intelmpi on Windows? After all Intel MPI is an MPICH derivative which is ABI compatible on Linux. That way an mpich value would work seamlessly in Linux, macOS, Windows.

That sounds like a good idea, but I've personally no idea/expertise on whether aliasing a derivative compiler as the foundation is good practice! I see now it's more nuanced than I first appreciated 🙏

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

No branches or pull requests

2 participants