Implement closer integration of AOCL components when using the amd-aocl package #43213

amd-toolchain-support · 2024-03-15T14:51:37Z

Implement closer integration of AOCL components when using the amd-aocl package

1) Implement closer integration of AOCL components when using amd-aocl package 2) Fix for aocl-sparse issue spack#43193

…d_sparse_issue

alalazo · 2024-03-18T09:46:35Z

var/spack/repos/builtin/packages/amdlibflame/package.py

+        default=True,
+        when="@4.1:",
+        description=(
+            "Enables tight coupling with AOCL-BLAS library in order to use AOCL-BLAS "
+            "internal routines"
+        ),


This variant is not needed, since we can make decisions based on:

spec.satisfies("^[virtuals=blas] amdblis")

internally.

we intend to use the new variants for enabling alternative code paths in the future (there is actually a new code path in libflame since 4.2) - they are not just for the purpose of managing dependencies.

@amd-toolchain-support Not sure what do you mean here. Can't we just do:

if spec.satisfies("@4.2: ^[virtuals=blas] amdblis"): args.append("-DAOCL_ROOT:PATH={0}".format(self.spec["blas"].prefix))

in the code below and avoid the variant? This variant seems to be used to select a specific virtual package for blas, and we don't usually do that (e.g. we don't have packages with an mpich variant that selects mpich as a provider for mpi).

In particular this makes it hard for a user to just say ^amdblis as they're used to doing to select a virtual -- what's the other code path you want to enable?

what's the other code path you want to enable?

Please have a look at https://github.com/amd/libflame/blob/b51ceb238bb3a597c014f371822fc6963b1d5893/CMakeLists.txt#L98
and then https://github.com/amd/libflame/blob/b51ceb238bb3a597c014f371822fc6963b1d5893/CMakeLists.txt#L988 to see how a CMake option AOCL_BLAS can be used to switch on a variable FLA_ENABLE_AOCL_BLAS. Here is an example of how FLA_ENABLE_AOCL_BLAS is being used in the LAPACK implementation to turn on a different code branch: https://github.com/amd/libflame/blob/b51ceb238bb3a597c014f371822fc6963b1d5893/src/lapack/dec/lu/piv/front/flamec/FLA_LU_piv_z.c#L557

Note that this branch will fail to work if end users provide an alternative BLAS through Spack. So @amd-toolchain-support is correct to say that the proposed change is "not just for the purpose of managing dependencies".

@ningli-amd I don't understand how this #43213 (comment) is not sufficient for what you say above. As far as I understand, you need to pass the correct options to CMake if amdblis is used.

Did you try to use:

if spec.satisfies("^[virtuals=blas] amdblis"):

in the recipe, as suggested above?

alalazo · 2024-03-18T09:47:10Z

var/spack/repos/builtin/packages/amdlibflame/package.py

+    requires("^amdblis", when="+enable-aocl-blas", msg="+enable-aocl-blas requires using amdblis")
+


Suggested change

requires("^amdblis", when="+enable-aocl-blas", msg="+enable-aocl-blas requires using amdblis")

var/spack/repos/builtin/packages/amdlibm/package.py

alalazo · 2024-03-18T09:50:52Z

var/spack/repos/builtin/packages/amdscalapack/package.py

+    variant(
+        "enable-aocl-blas-lapack",
+        default=True,
+        when="@4.1:",
+        description=(
+            "Enables tight coupling with AOCL BLAS/LAPACK library in order to use AOCL "
+            "internal routines"
+        ),
+    )


Same as above: the variant is not needed, since we can take decisions internally based on the providers of blas and lapack.

alalazo · 2024-03-18T09:51:25Z

var/spack/repos/builtin/packages/amdscalapack/package.py

+    requires(
+        "^amdblis",
+        when="+enable-aocl-blas-lapack",
+        msg="+enable-aocl-blas-lapack requires using amdblis",
+    )
+    requires(
+        "^amdlibflame",
+        when="+enable-aocl-blas-lapack",
+        msg="+enable-aocl-blas-lapack requires using amdlibflame",
+    )


Suggested change

requires(

"^amdblis",

when="+enable-aocl-blas-lapack",

msg="+enable-aocl-blas-lapack requires using amdblis",

)

requires(

"^amdlibflame",

when="+enable-aocl-blas-lapack",

msg="+enable-aocl-blas-lapack requires using amdlibflame",

)

var/spack/repos/builtin/packages/aocl-sparse/package.py

alalazo · 2024-03-18T09:53:24Z

var/spack/repos/builtin/packages/amdscalapack/package.py

        args.extend(
            [
-                "-DLAPACK_FOUND=true",
+                "-DUSE_OPTIMIZED_LAPACK_BLAS=true",


What is this change about?

We decided to switch to an alternative BLAS/LAPACK detection mechanism in amdscalapack's CMake build system.

Does this work with previous versions of amdscalapack or should we use a different option based on versions?

alalazo · 2024-03-18T09:56:30Z

Can you also please improve the title? "Following changes were done for AOCL libraries:" is not really representative of what the PR is doing.

…nts when using the amd-aocl package

amd-toolchain-support · 2024-03-19T04:26:48Z

Can you also please improve the title? "Following changes were done for AOCL libraries:" is not really representative of what the PR is doing.

Done

…d_sparse_issue

amd-toolchain-support · 2024-05-21T14:34:33Z

We are writing to discuss a challenge we are encountering with the use of 'spack virtuals' as opposed to the 'enable-aocl-blas' variant in amdlibflame and the 'enable-aocl-blas-lapack' variant in amdscalapack.

While the benefits of 'spack virtuals' are clear, we have found that when installing amdlibflame or amdscalapack from a fresh workspace, the spack concretizer defaults to pulling openblas. This is not our intended result, as we aim to ensure the installation of AMD products exclusively.

We can achieve this within the amd-aocl spack bundle using the commands:

'depends_on(f"amdlibflame@={vers} ^[virtuals=blas] amdblis")' and 'depends_on(f"amdscalapack@={vers} ^[virtuals=blas] amdblis ^[virtuals=lapack] amdlibflame")'

However, replicating this process for HPC applications installation is inefficient and cumbersome. As such, we propose the use of 'variant' as a more practical solution for our needs.

We have attached the spec results for your reference. We welcome further discussions and suggestions on this matter. Thank you for your understanding and continued support.

spack_amdscalapack_spec_output_with_virtuals.txt
spack_amdscalapack_spec_output.txt

alalazo · 2024-05-22T08:00:59Z

This is not our intended result, as we aim to ensure the installation of AMD products exclusively.

You can use requirements for that: https://spack.readthedocs.io/en/latest/packages_yaml.html#setting-requirements-on-virtual-specs

amd-toolchain-support · 2024-05-23T13:00:52Z

Hi @alalazo, @tgamblin,

I realise that we've not really explained very well what we are trying to achieve with our AOCL Spack support. Please allow me to try and give the full picture here, so that we can try to find something that will hopefully work for everyone.

Currently we do not ship a monolithic library recipe because we are aware that some users do value being able to mix-and-match blas and lapack providers. We also have users who would like to use the full AMD stack, and be able to install it via the amd-aocl metapackage. This is why our historically preferred solution has been the per-lib recipes plus a metapackage.

The problem we are running into is that following the provider rework from a few months ago, we are not able to encode the metapackage dependency requirements to ensure that a full AOCL-only stack is built. We are instead finding that no matter what we do, OpenBLAS sneaks into the dep graph, usually as a dependency of amdlibflame. That results in having two blas and two lapack providers in the same spec, with implementation details determining which routines actually get called at runtime.

So with that in mind, the reason we have introduced the variants is to try to force the concretizer to give us what want when building the amd-aocl metapackage. If we can solve that dependency problem without those variants, then I think that the implementation route that you have suggested will work without issue.

Below is a small example that boils down what we are trying to achieve in the metapackage to a minimal example. If you can help us understand how to encode the requirements in mymetapackage to ensure that blasuserpackage always selects amdblis as the provider, then we can use that logic in our amd-aocl metapackage. Currently I cannot find a valid expression of this that the concretizer will accept and honor.

I realise the user can provide install-time dependency statements to acheive this control, but we would like to find a way of ensuring that the metapackage behaves correctly for the simple spack install amd-aocl case.

We would appreciate your input on how best to achieve this.

class Mymetapackage(BundlePackage):
    """Test package for concretization"""
    
    homepage = "https://www.example.com"
    url = "mymetapackage"

    version("0.1.2")

    depends_on("blasuserpackage ^[virtuals=blas] amdblis") # What goes here?
    requires("blasuserpackage ^[virtuals=blas] amdblis") # and what goes here?

class Blasuserpackage(MakefilePackage):
    """Test package consuming blas"""

    homepage = "https://www.example.com"
    url = "blasuserpackage"

    version("0.1.2")

    depends_on("blas")

Following changes were done for AOCL libraries:

efb1459

1) Implement closer integration of AOCL components when using amd-aocl package 2) Fix for aocl-sparse issue spack#43193

spackbot-app bot added dependencies new-variant update-package labels Mar 15, 2024

amd-toolchain-support requested a review from tldahlgren March 15, 2024 14:51

amd-toolchain-support added the AMD label Mar 15, 2024

Merge branch 'develop' into aocl/libflame_blis_tighter_integration_an…

df0aa90

…d_sparse_issue

amd-toolchain-support requested a review from alalazo March 16, 2024 04:46

fix multi-threading support for amdlibflame v4.2

4b8290d

alalazo requested changes Mar 18, 2024

View reviewed changes

alalazo self-assigned this Mar 18, 2024

Limiting the changes for Implement closer integration of AOCL compone…

43a3b1d

…nts when using the amd-aocl package

amd-toolchain-support changed the title ~~Following changes were done for AOCL libraries:~~ Implement closer integration of AOCL components when using the amd-aocl package Mar 19, 2024

amd-toolchain-support added 2 commits March 29, 2024 07:56

Merge branch 'develop' into aocl/libflame_blis_tighter_integration_an…

da56bc6

…d_sparse_issue

Merge branch 'develop' into aocl/libflame_blis_tighter_integration_an…

acbb9b2

…d_sparse_issue

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement closer integration of AOCL components when using the amd-aocl package #43213

Implement closer integration of AOCL components when using the amd-aocl package #43213

amd-toolchain-support commented Mar 15, 2024 •

edited

Loading

alalazo Mar 18, 2024

amd-toolchain-support Mar 19, 2024

alalazo Mar 28, 2024

tgamblin May 22, 2024

ningli-amd May 22, 2024

alalazo May 22, 2024

alalazo Mar 18, 2024

alalazo Mar 18, 2024

alalazo Mar 18, 2024

alalazo Mar 18, 2024

ningli-amd Mar 18, 2024

alalazo Mar 28, 2024

alalazo commented Mar 18, 2024

amd-toolchain-support commented Mar 19, 2024

amd-toolchain-support commented May 21, 2024 •

edited

Loading

alalazo commented May 22, 2024

amd-toolchain-support commented May 23, 2024

		requires("^amdblis", when="+enable-aocl-blas", msg="+enable-aocl-blas requires using amdblis")

Implement closer integration of AOCL components when using the amd-aocl package #43213

Are you sure you want to change the base?

Implement closer integration of AOCL components when using the amd-aocl package #43213

Conversation

amd-toolchain-support commented Mar 15, 2024 • 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

alalazo commented Mar 18, 2024

amd-toolchain-support commented Mar 19, 2024

amd-toolchain-support commented May 21, 2024 • edited Loading

alalazo commented May 22, 2024

amd-toolchain-support commented May 23, 2024

amd-toolchain-support commented Mar 15, 2024 •

edited

Loading

amd-toolchain-support commented May 21, 2024 •

edited

Loading