A path to making OpenBLAS optarch aware. #1946

terjekv · 2020-02-06T11:37:51Z

A first and very raw implementation, mapping optarch's march
directly to targets known to OpenBLAS.
A better mapping is required. Skylake-avx512 vs SKYLAKEX as
an obvious example.

easybuild/easyblocks/o/openblas.py

terjekv · 2020-09-06T18:49:39Z

@boegel and @tgamblin, I'm trying to return to resolving the lack of optarch support for OpenBLAS, but in a slightly more generic fashion. @boegel asked if leaning on https://github.com/archspec/archspec would be a good idea, and that most certainly seems like a very good idea indeed. However, there are a few thoughts on how to proceed:

My current thinking is that the goal should be to map an architecture specification, be it GCCs nomenclature, OpenBLAS' own, or something completely different, to a label from archspec. From there we can thus "translate" from one naming scheme to another at will, using archspec as our Rosetta Stone,
The question then becomes one of implementation. Should these distinct nomenclatures be integrated as "plugins" or something similar to archspec, or should these individual libraries be "consumers" of archspec, remaining completely distinct from the central project? Separation removes clutter, but it require more "bits" to be installed in the form of distinct libraries, and this will be something the user will have to maintain (or optionally EasyBuild?)
Versioning of a given local nomenclature to archspec mappings. Archspec is already familiar with how to make certain supported compilers optimise for a given architecture, and what versions of the compilers are required. But, we would need an even broader mapping for applications. The OpenBLAS TargetList both adds and renames targets. This implies that a correct mapping will have to be release-dependent.
As a side note to this work... If we get this working, someone needs to decide the extent of EasyBuild's optarch support. Long term, support for cross-compiling will probably become a question that will require a definitive answer.

tgamblin · 2020-09-06T19:25:11Z

Did you look at the Spack openblas package? The mapping from archspec to openblas is here:

https://github.com/spack/spack/blob/develop/var/spack/repos/builtin/packages/openblas/package.py#L141

It's close enough that we just search for a target based on the archspec name, with a few special cases.

Given that OpenBLAS is one package, I'm not inclined to add its names to archspec -- I think the translation belongs in the package file. If the goal is to avoid duplicating that work for lots of package managers, I'd say we should try to upstream archspec support to openblas -- it's already pretty close so they might go for something like this.

terjekv · 2020-09-06T19:54:32Z

Hi @tgamblin and thank you for your input! I poked at at the Spack implementation and it looks very smooth. I feel there needs to be a bit of version checking on the TargetList, but that's another problem all together.

As you say, cluttering the main archspec JSON is a bad idea, which is why I thought of secondary mappings or "plugins" to provide extended support. There are also cases beyond OpenBLAS that will eventually need addressing. However, in the short term, using archspec and the Spack implementation as a reference would solve the immediate issue, and then we can take it from there. Hm.

Thanks again for your input, very helpful!

(And if you can get OpenBLAS to be a little bit more conformant, that'd be amazing.)

Flamefire · 2020-09-30T11:43:39Z

easybuild/easyblocks/o/openblas.py

+
+        for piece in pieces:
+            spec = piece.split('=')
+            if spec[0] == 'march' or spec[0] == '-march':


What about -mcpu? I see that used on POWER

-mcpu is used, but it is a deprecated synonym for -mtune, which only takes two values on x86, generic and Intel, neither of which have an effect on OpenBLAS targeting, which only cares about CPU architecture. I have no idea what this option does on Power. :(

See https://stackoverflow.com/questions/42718572/gcc-mtune-vs-march-vs-mcpu

-mcpu unfortunately has different semantics for different targets. It's deprecated for x86 (being a synonym for -mtune) but not for ARM, where it's a sum of -march and -mtune.

Flamefire · 2020-09-30T11:45:46Z

easybuild/easyblocks/o/openblas.py

+                              % (self.toolchain.comp_family(), compiler_optarch))
+                self.log.warning("Unable to map %s to an OpenBLAS target, falling back to runtime optimization."
+                                 % compiler_optarch)
+                self._set_dynamic_architecture(default_opts)


I'm very unsure about this. If I build OpenBLAS currently, it seems to autodetect the current arch and optimizes for it. It seems to even overwrite our optarch settings etc which is fine when building for native. Additionally e.g. for POWER and OpenBLAS until 0.3.7 (IIRC) DYNAMIC_ARCH is broken to the point where even the simple tests fail (so in this case the EC wouldn't build)

Ah, thanks! With DYNAMIC_ARCH being broken on POWER, we should certainly it for Power.

But the thing is, if you ask for optarch=haswell and OpenBLAS optimises for native, and you're on a skylake build host, your compute nodes will cease to function. For me (and I presume others), optarch is not something that may be ignored.

My strong preference is that if an EasyBuild user sets optarch=haswell EasyBuild offers the guarantee that the code produced will indeed run on haswell. If this can't be done for an EasyConfig, it is, at least for me, vastly preferable for the EasyConfig to fail during build.

Alright sure. I just wanted to highlight that the default (native) should still work as before (hence my restriction to "when building for native")

Flamefire · 2020-09-30T11:47:05Z

easybuild/easyblocks/o/openblas.py

+
+        return target_arch.lower()
+
+    def _get_openblas_targets(self, targetfile):


For all those functions: Could you make them free functions? In PyTorch/TensorFlow I found it a good idea so you can just import this from regular python and run/test it

@lexming (I think) suggested generalising a number of these functions and shipping them off to Framework. The ones that are OpenBLAS specific should probably suffer a similar fate. This will require another rewrite though, not sure about the ETA on that.

Flamefire · 2020-09-30T11:47:34Z

easybuild/easyblocks/o/openblas.py

+            if compiler in optarch:
+                compiler_specific_optarch_string = optarch[compiler]
+        else:
+            raise EasyBuildError("optarch in an unexpected format: '%s'" % type(optarch)).__class__.__name__


Please remove the .__class__.__name__. Also maybe include the optarch value and use , instead of %

Flamefire · 2020-09-30T11:48:18Z

easybuild/easyblocks/o/openblas.py

+        compiler = self.toolchain.comp_family()
+        compiler_specific_optarch_string = ''
+
+        if type(optarch) == str:


you can (and IIRC should) use isinstance

Thanks, will do.

Flamefire · 2020-09-30T11:54:08Z

easybuild/easyblocks/o/openblas.py

+        # strip march from all environment variables except the EBVAR-prefixed ones.
+        # For dynamic builds we should ignore optarch completely and for optarch-set builds
+        # we need to adhere to TARGET and not march.
+        if self._dynamic_target is True or self._optarch_architecture is True:


In which case can self._optarch_architecture be True? It is either False or a string isn't it?

Hm, yeah. I think this was a suggested change from the editor of best practices for python. There was something odd about that. I'll check it out.

Flamefire · 2020-09-30T11:54:51Z

easybuild/easyblocks/o/openblas.py

+        :param optarch: A complete optarch statement.
+        https://easybuild.readthedocs.io/en/latest/Controlling_compiler_optimization_flags.html
+        """
+        if optarch is False:


For me (on POWER) optarch is None, so maybe if not optarch:? And I'd return None too as that seems to better fit the meaning

Yep, certainly.

Flamefire · 2020-09-30T11:55:50Z

easybuild/easyblocks/o/openblas.py

+            if uarch.name in available_targets:
+                result = uarch.name
+            else:
+                self.log.info("No direct match for '" + march + "' between archspec and OpenBLAS, traversing ancestry.")


Don't we use formatted logs instead of string concatting?

Probably? I'll have to look at that.

boegel · 2023-06-15T09:13:56Z

@terjekv I think you'll agree with me that it's unlikely that this PR will get merged.

It was at least partially motivated by a need in EESSI, and we've started taking a different approach there in EESSI/software-layer#260 by implementing a custom hook for OpenBLAS which injects DYNAMIC_ARCH=1 when using --optarch=GENERIC.

That could/should probably also be implemented in the OpenBLAS easyblock, but in a different way than is being done here - this PR does way more than just injecting DYNAMIC_ARCH=1.

terjekv · 2023-06-23T11:56:53Z

I'd wager it could be merged if I rewrote it to use DYNAMIC_ARCH in a sane way when seeing optarch=GENERIC, but hooks work fine for now. So fine I've been using it for long while myself.

Close and let a new, maybe working, PR arise if one ha the energy. :)

houndci-bot reviewed Feb 6, 2020

View reviewed changes

boegel requested changes Feb 6, 2020

View reviewed changes

easybuild/easyblocks/o/openblas.py Outdated Show resolved Hide resolved

easybuild/easyblocks/o/openblas.py Outdated Show resolved Hide resolved

easybuild/easyblocks/o/openblas.py Outdated Show resolved Hide resolved

boegel added the enhancement label Feb 6, 2020

boegel added this to the next release (4.1.2?) milestone Feb 6, 2020

houndci-bot reviewed Feb 10, 2020

View reviewed changes

easybuild/easyblocks/o/openblas.py Outdated Show resolved Hide resolved

terjekv requested a review from boegel February 10, 2020 20:19

houndci-bot reviewed Feb 12, 2020

View reviewed changes

terjekv marked this pull request as ready for review February 27, 2020 11:42

boegel modified the milestones: next release (4.2.0), release after 4.2.0 (4.2.1?) Apr 12, 2020

boegel modified the milestones: 4.2.1, release after 4.2.1 (4.2.2?) May 19, 2020

boegel modified the milestones: 4.2.2, release after 4.2.2 (4.2.3?) Jul 3, 2020

terjekv mentioned this pull request Jul 28, 2020

OpenBLAS/0.3.7-GCC-8.3.0 with optarch=GENERIC builds with avx512. easybuilders/easybuild-easyconfigs#9754

Open

boegel modified the milestones: next release (4.3.0), release after 4.3.0 (4.3.1?) Sep 2, 2020

terjekv closed this Sep 9, 2020

terjekv force-pushed the openblas-oparch-support branch from 951167d to 9885496 Compare September 9, 2020 08:57

Sync branch with current develop. Properly.

047619d

terjekv reopened this Sep 9, 2020

Actually use the package wide TARGET variable.

cc74e45

This was referenced Sep 18, 2020

OpenBLAS cannot be installed easybuilders/easybuild-easyconfigs#11306

Closed

add easyblock for RAxML #2180

Open

Flamefire suggested changes Sep 30, 2020

View reviewed changes

boegel modified the milestones: 4.3.1 (next release), 4.3.2 Oct 25, 2020

boegel added this to the release after 4.4.2 milestone Sep 1, 2021

boegel modified the milestones: 4.5.0 (next release), release after 4.5.0 Oct 27, 2021

boegel modified the milestones: 4.5.1, release after 4.5.1 Dec 7, 2021

boegel modified the milestones: 4.5.2, release after 4.5.2 Jan 14, 2022

boegel modified the milestones: 4.5.3, release after 4.5.3 Feb 9, 2022

boegel modified the milestones: next release (4.5.4), release after 4.5.4 Mar 25, 2022

boegel modified the milestones: 4.5.5, release after 4.5.5 Jun 4, 2022

boegel modified the milestones: next release (4.6.0), release after 4.6.0 Jul 6, 2022

boegel modified the milestones: next release (4.6.1), release after 4.6.1 Sep 9, 2022

boegel added the EESSI Related to EESSI project label Oct 18, 2022

boegel modified the milestones: next release (4.6.2?), release after 4.6.2 Oct 18, 2022

boegel modified the milestones: next release (4.7.0), release after 4.7.0 Dec 20, 2022

boegel modified the milestones: next release (4.7.1), release after 4.7.1 Mar 1, 2023

boegel modified the milestones: next release (4.7.2), release after 4.7.2 May 10, 2023

boegel mentioned this pull request Jun 6, 2023

add OpenFOAM v9 to EESSI pilot 2021.12 EESSI/software-layer#195

Closed

10 tasks

terjekv closed this Jun 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

A path to making OpenBLAS optarch aware. #1946

A path to making OpenBLAS optarch aware. #1946

terjekv commented Feb 6, 2020

terjekv commented Sep 6, 2020 •

edited

Loading

tgamblin commented Sep 6, 2020

terjekv commented Sep 6, 2020 •

edited

Loading

Flamefire Sep 30, 2020

terjekv Oct 2, 2020

Flamefire Oct 5, 2020

Flamefire Sep 30, 2020

terjekv Oct 2, 2020 •

edited

Loading

Flamefire Oct 5, 2020

Flamefire Sep 30, 2020

terjekv Oct 2, 2020

Flamefire Sep 30, 2020

Flamefire Sep 30, 2020

terjekv Oct 2, 2020

Flamefire Sep 30, 2020

terjekv Oct 2, 2020

Flamefire Sep 30, 2020

terjekv Oct 2, 2020

Flamefire Sep 30, 2020

terjekv Oct 2, 2020

boegel commented Jun 15, 2023

terjekv commented Jun 23, 2023


		return target_arch.lower()

		def _get_openblas_targets(self, targetfile):

A path to making OpenBLAS optarch aware. #1946

A path to making OpenBLAS optarch aware. #1946

Conversation

terjekv commented Feb 6, 2020

terjekv commented Sep 6, 2020 • edited Loading

tgamblin commented Sep 6, 2020

terjekv commented Sep 6, 2020 • edited Loading

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

terjekv Oct 2, 2020 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boegel commented Jun 15, 2023

terjekv commented Jun 23, 2023

terjekv commented Sep 6, 2020 •

edited

Loading

terjekv commented Sep 6, 2020 •

edited

Loading

terjekv Oct 2, 2020 •

edited

Loading