conda-forge / mpich-feedstock

A conda-smithy repository for mpich.
BSD 3-Clause "New" or "Revised" License
2 stars 26 forks source link

Enable Fortran 2008 support #84

Closed aradi closed 9 months ago

aradi commented 11 months ago

Checklist

Adds support for the Fortran 2008 interface for MPICH in Conda. Now, after the appropriate fix in the GNU-compiler (conda-forge/ctng-compilers-feedstock#123), this finally seems to be possible. This PR explicitly enables the F2008 interface build and tests the F2008 interface.

conda-forge-webservices[bot] commented 11 months ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

aradi commented 11 months ago

@conda-forge-admin, please rerender

aradi commented 9 months ago

Note, that for os osx_64_mpi_typeconda the CI fails. The problem seems to be the failing detection of the Fortran 2008 capacity of the compiler in the configure script. As far as I understand, this check basically tries to compile a minimal C-program, which includes the ISO_Fortran_binding.h file. This file should be provided by the compiler. Not sure, why this does not work for osx_64 (see also https://github.com/conda-forge/gfortran_impl_osx-64-feedstock/issues/78).

dalcinl commented 9 months ago

@aradi I believe my hotfix may have solved the issue. However, what I'm doing here is a nasty hack that should be fixed elsewhere. If you care about maintaing Fortran 2008 support in this feedstock, then you should follow up with the gfortran maintainers to figure out the proper way to address the clang+gfortran mix up in macOS. If things are not properly addressed where they belong and the hack I've added gives me any maintenance trouble in the future, I will not hesitate in rolling back and disabling F08 support.

The issue is still there in osx-arm64. We just don't get the test failure simply because tests are not run on macOS when cross-compiling. If things go well with with osx-64, I'll try to extend the workaround for osx-arm64.

aradi commented 9 months ago

@dalcinl Thanks for the hotfix. So, that means, that the file is there, it is just not found by the C-compiler (I've completely missed the fact, that osx uses clang instead of gcc).

Two comments from my side:

aradi commented 9 months ago

I've tried an alternative approach with explicitly setting the proper include path via the -I option for the C and C++ compilers. We'll see in ca. 2 hours, how it behaves. If it fails, I'll simply revert it. (If it works, we should probably consider to use this approach instead of symlinking.)

dalcinl commented 9 months ago

@aradi Sorry, I missed the outcome of my last commit. Did it work? Can you explain the purpose of your last commit?

dalcinl commented 9 months ago

Oh, I see what you are doing, and I don't like it. You are injecting the include directory for all platforms, but so far the issue seems to affect only macOS, therefor we should not mess with Linux if not needed. Moreover, it is not MPICH's responsibility to workaround deficiencies in in the compiler packages. IMHO, the issue has to be addressed in the gfortran feedstock, not here.

aradi commented 9 months ago

OK, I'll revert it, as it unfortunately messes up clangs and gcc's include files on OSX.

aradi commented 9 months ago

Just some explanations:

dalcinl commented 9 months ago

@aradi Could you please open an issue in the gfortran feedstock and CC me? I believe the fix should be easy, we just need to create a lib/clang folder and symlink the header in there.

EDIT: Although that's not so easy, as the folder depends on the clang version.

dalcinl commented 9 months ago
  • I still do not think, that it is the compiler packages responsibility to fix the problem

But then would mean means that, at least on macOS's current setup within conda-forge based on clang+gfortran, you cannot really use ISO_Fortran_binding.h out of the box. What a pity!

The ISO_Fortran_binding.h header is provided by the Fortran compiler for the consumption of the C compiler. If the two compilers come from different vendors and you want to use them together in an environment like conda, then I would argue that it would make sense to set things up such that clang can automatically find the ISO_Fortran_binding.h without explicit user intervention.

aradi commented 9 months ago

EDIT: Although that's not so easy, as the folder depends on the clang version.

I think, we should treat Fortran and C compilers as independent entities, and not try to let one of them to mess around with the include paths of the other. So, the question is, what is the best solution for C projects, which wish to use the Fortran interface, to find ISO_Fortran_binding.h. Currently, unfortunately, your hack seems to me the best solution (it worked for OSX_64, sorry for overwriting it too early...).

aradi commented 9 months ago

One possibilty would be to create a temporary include directory in user space, copy / symlink ISO_Fortran_binding.h there, and add that include directory (containing a single .h only) to MPICH_CFLAGS. The effect would be similar to your fix, but it won't write in clangs include directory...

aradi commented 9 months ago

If you like that idea, I am happy to implement it and check that in. But we can also stay with your (somewhat simpler) fix, if you prefer.

dalcinl commented 9 months ago

I believe that for the purposes of building MPICH with Fortran 2008 support, the hack we have now in place is good enough and likely future proof. I do not care at all about writing to the clang dir in the build recipe. All that happens in a scratch build environment that gets eventually deleted.

The more general issue of using ISO_Fortran_binding.h with clang on macOS is a different story. We should not attempt to fix that thing here. If you need such thing for your own projects, then any fix should be discussed in the gfortran feedstock.

aradi commented 9 months ago

@dalcinl OK, great, the let's go with the fix as it is now. Thanks for your help and the discussions.

aradi commented 9 months ago

@dalcinl I've just checked, the fix is still not 100%-ly correct, as it does not work for osx_arm64 yet. Looking at the output of the CI of the merge of this PR to main, the Fortran 2008 support is still not detected on osx_arm64 (2024-01-30T10:26:36.0136360Z checking for Fortran 2008 support... no). For osx_64 it works, as expected. Any clues how to fix it?

dalcinl commented 9 months ago

I'm a bit busy right now. Could you please submit a PR with cat config.log && exit after configure such that we can see what's really going on? IIRC, I tried things manually in bare metal and it worked just fine. In retrospective, I should have tried the build-locally.py script instead :disappointed:.