Mpi omp support #14

hiker · 2024-08-02T10:08:01Z

Adds support for specifying MPI and OpenMP in the config file.

while it works with the tests, a real application still triggered it. This reverts commit 150dc37.

…Fixed coding style.

…nfig.

…python 3.7.

hiker · 2024-08-02T10:16:34Z

This patch is not 'as big' as it appears to be - because of adding parameters to BuildConfig, they had to be added to a lot of files and examples. Additionally, I addressed coding style issues as suggested by Matthew recently (max 80 characters/line). Note that the MPI based compiler classes are not proper wrapper yet, they will be re-implemented in the next PR.

hiker · 2024-08-08T01:44:54Z

Quick explanation:
the idea of this PR is that the user defines in the BuildConfig if the build is to be done with MPI, and with OpenMP (they are not exclusive). This means that the ToolRepository can pick an appropriate compiler by default (if the user doesn't specify what compiler they want), which solves e.g. the problem that e.g. ifort is being picked as default for an intel-based build (instead of an Intel based MPI wrapper). ATM we hard-code this still in the script.
The support for OpenMP means that the compiler can select its own (compiler-specific) OpenMP flags. ATM we have these flags hard-coded in the scripts.

source/fab/tools/compiler.py

lukehoffmann · 2024-08-09T06:41:43Z

source/fab/tools/tool_box.py

@@ -59,6 +61,9 @@ def get_tool(self, category: Category) -> Tool:
            return self._all_tools[category]

        # No tool was specified for this category, get the default tool
-        # from the ToolRepository:
+        # from the ToolRepository, and at it, so we don't need to look


and at it

do you mean "and add it"?

tests/unit_tests/tools/test_compiler.py

lukehoffmann · 2024-08-09T07:48:29Z

tests/unit_tests/tools/test_compiler.py

+    assert mpi_gfortran.name == "mpif90-gfortran"
+    assert isinstance(mpi_gfortran, FortranCompiler)
+    assert mpi_gfortran.category == Category.FORTRAN_COMPILER
+    assert mpi_gfortran.mpi



I think we need a test of get_version() on the mpi compilers, to make sure we fixed the issue Jason found

I've parameterised all the valid compiler version tests to test both the compiler and mpi wrapper.

In hindsight a stupid idea of me. The next PR (compiler wrapper) refactors the MPI* wrapper and creates their own file with their own tests, so I had to remove the parameterisation in the next PR again. The 'future' compiler wrapper do explicitly test that the wrapper and the wrapper compiler report the same version :(

tests/unit_tests/tools/test_tool_repository.py

lukehoffmann · 2024-08-12T03:18:06Z

tests/unit_tests/tools/test_compiler.py

    # Notice that "-J/b" has been removed
    fc.run.assert_called_with(cwd=PosixPath('.'),
                              additional_parameters=['-c', "-O3",
                                                     '-J', '/module_out',
                                                     'a.f90', '-o', 'a.o'])
+    with pytest.warns(UserWarning,


I'd prefer this to be a separate test, but it at least needs a comment. Currently you have to play spot-the-difference with the code above 😀

Done ... I think.

tests/unit_tests/tools/test_linker.py

source/fab/tools/tool_box.py

lukehoffmann · 2024-08-12T05:01:01Z

source/fab/build_config.py

@@ -164,9 +184,20 @@ def build_output(self) -> Path:
        '''
        return self.project_workspace / BUILD_OUTPUT

+    @property
+    def mpi(self) -> bool:


Are these properties (BuildConfig.mpi and BuildConfig.openmp) used in you later tickets? Right now I can't see if they do anything. Couldn't I create a BuildConfig(mpi=False) and still use it with mpi?

Good catch. MPI was used in compile_c, but not compile_fortran (and as previously discussed, we don't raise an exception if you request MPI and a non-MPI compiler is already loaded, based on the idea that the user knows what they are doing). I am thinking more and more that we should raise an exception in this case :(
Openmp is used in both compile_c and ..._fortran

lukehoffmann · 2024-08-12T05:15:19Z

source/fab/tools/tool_repository.py

+            # the key being tool.suite != suite --> all tools with the right
+            # suite use False as key, all other tools True. Since False < True
+            # this results in all suite tools to be at the front of the list
+            self[category] = sorted(self[category],


Does this assume add_tool is never called after set_default_compiler_suite? That might be fine, I can't tell

Not really. ATM I use this (in the NCI setup) to first pick our default suite (intel-classic), then add additional compilers (tau-wrapper around ifort) to the repository. This way the default will still remain the plain ifort (or mpi wrapper around ifort), while the tau ones (which are all of the same suite 'intel-classic') are available if a user wants to pick them, but they will not be picked by default.

Actually, now that I think about it, the order doesn't matter. Fab will first add the 'standard' compiler (e.g. ifort, mpif90-ifort), and whenever the default is changed, a stable-sort is called, so ifort/mpif90-ifort will be before any user added tool.

hiker · 2024-08-14T13:50:55Z

I hope I've addressed all issues. Ready for next review.

source/fab/tools/compiler.py

source/fab/tools/tool_box.py

hiker added 14 commits July 24, 2024 15:06

Add mpi and openmp settings to BuildConfig, made compiler MPI aware.

90adcca

Looks like the circular dependency has been fixed.

150dc37

Revert "Looks like the circular dependency has been fixed." ...

db34597

while it works with the tests, a real application still triggered it. This reverts commit 150dc37.

Don't even try to find a C compiler if no C files are to be compiled.

7c347c8

Updated gitignore to ignore (recently renamed) documentation.

ff71393

Fixed failing test.

5dc01f3

Return from compile Fortran early if there are no files to compiles. …

2b5c4bd

…Fixed coding style.

Add MPI enables wrapper for intel and gnu compiler.

46706c5

Fixed test.

d55344f

Automatically add openmp flag to compiler and linker based on BuildCo…

d00c187

…nfig.

Removed enforcement of keyword parameters, which is not supported in …

e9333fd

…python 3.7.

Fixed failing test.

a85bc7a

Support more than one tool of a given suite by sorting them.

6c8e8df

Merge branch 'bom_master' into mpi_omp_support

196e034

Merge branch 'version_tuples' into mpi_omp_support

20fef2c

hiker added 2 commits August 9, 2024 23:14

Merge branch 'version_tuples' into mpi_omp_support

1524777

Merge remote-tracking branch 'upstream/master' into mpi_omp_support

b38cdb8

lukehoffmann requested changes Aug 12, 2024

View reviewed changes

hiker added 9 commits August 13, 2024 21:27

Merge remote-tracking branch 'upstream/master' into version_tuples

6826c85

Merge branch 'version_tuples' into mpi_omp_support

e5aca3b

Split tests into smaller individual ones, fixed missing asssert in test.

ccbe810

Parameterised compiler version tests to also test wrapper.

f8f8ab0

Added missing MPI parameter when getting the compiler.

1543841

Fixed comments.

cd8ec62

Order parameters to be in same order for various compiler classes.

13935c0

Merge branch 'bom_master' into mpi_omp_support

de80c44

Merge remote-tracking branch 'origin/bom_master' into mpi_omp_support

009478f

hiker requested a review from lukehoffmann August 14, 2024 13:51

Remove stray character

7887d24

lukehoffmann approved these changes Aug 16, 2024

View reviewed changes

source/fab/tools/compiler.py Show resolved Hide resolved

source/fab/tools/tool_box.py Show resolved Hide resolved

lukehoffmann merged commit 83836e7 into bom_master Aug 16, 2024
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mpi omp support #14

Mpi omp support #14

hiker commented Aug 2, 2024

hiker commented Aug 2, 2024

hiker commented Aug 8, 2024

lukehoffmann Aug 9, 2024

hiker Aug 14, 2024

lukehoffmann Aug 9, 2024

hiker Aug 14, 2024

hiker Aug 14, 2024

lukehoffmann Aug 12, 2024

hiker Aug 14, 2024

lukehoffmann Aug 12, 2024

hiker Aug 14, 2024

lukehoffmann Aug 12, 2024

hiker Aug 14, 2024

hiker commented Aug 14, 2024

Mpi omp support #14

Mpi omp support #14

Conversation

hiker commented Aug 2, 2024

hiker commented Aug 2, 2024

hiker commented Aug 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hiker commented Aug 14, 2024