[icon] Adds variant 'gpu' #782

dominichofer · 2023-06-30T12:44:57Z

The variant "gpu" follows closer the configure of icon.
This change is required to use --enable-gpu=openacc for the configure script in ICON

github-actions · 2023-06-30T12:45:34Z

PR Preview Action v1.4.4
Preview removed because the pull request was closed.
2023-07-13 15:27 UTC

lxavier · 2023-07-04T08:41:45Z

when I compile icon I would like to have only: +gpu=openacc+cuda would this be the case, or do I also need to set +cuda ?

dominichofer · 2023-07-04T09:35:17Z

+gpu=openacc+cuda conflicts with ~cuda, so it should cause +cuda automatically.

lxavier · 2023-07-04T10:10:00Z

+gpu=openacc+cuda conflicts with ~cuda, so it should cause +cuda automatically.

ok , is this the spack behavior, is not going to give an error because default is ~cuda ?
Should we not have a "depends_on" like depends_on(cuda,when='openacc+cuda' ?

dominichofer · 2023-07-04T11:26:54Z

No, no error.
depends_on('cuda', when='gpu=openacc+cuda') would only make the package depend on the cuda library. The variant cuda would be unaffected. But the variant 'cuda_arch' in spack.CudaPackage is only available when +cuda. Therefore I think it's simpler to make the variant cuda follow the variant gpu to leverage spack.CudaPackage's functionality.

dominichofer · 2023-07-04T11:42:42Z

launch jenkins icon

jenkins-apn · 2023-07-04T11:47:31Z

tsa

🟢 unit test

	Test
🟢	summary

🟢 integration test

	Test
🟢	icon-spack_info
🟢	icon-spack_spec
🟢	icon_serialization=create_claw=std-spack_spec

jenkins-apn · 2023-07-04T11:57:30Z

balfrin

🟢 unit test

	Test
🟢	summary

🟢 integration test

	Test
🟢	icon-spack_info
🟢	icon-spack_spec
🟢	icon_serialization=create_claw=std-spack_spec

🟢 system test

	Test
🟢	Icon-install_nwp_cpu
🟢	Icon-install_nwp_gpu

jenkins-apn · 2023-07-04T13:00:57Z

daint

🟢 unit test

	Test
🟢	summary

🟢 integration test

	Test
🟢	icon-spack_info
🟢	icon-spack_spec
🟢	icon_serialization=create_claw=std-spack_spec

🔴 system test

	Test
🔴	daint_cpu_cce
🟢	daint_cpu_gcc
🟢	daint_cpu_nvhpc
🟢	daint_cpu_nvhpc_out_of_source
🟢	daint_gpu_nvhpc

WARNING: Serial tests did not run for system tests

dominichofer · 2023-07-05T07:03:49Z

launch jenkins icon

jenkins-apn · 2023-07-05T07:09:17Z

tsa

🟢 unit test

	Test
🟢	summary

🟢 integration test

	Test
🟢	icon-spack_info
🟢	icon-spack_spec
🟢	icon_serialization=create_claw=std-spack_spec

jenkins-apn · 2023-07-05T07:20:05Z

balfrin

🟢 unit test

	Test
🟢	summary

🟢 integration test

	Test
🟢	icon-spack_info
🟢	icon-spack_spec
🟢	icon_serialization=create_claw=std-spack_spec

🟢 system test

	Test
🟢	Icon-install_nwp_cpu
🟢	Icon-install_nwp_gpu

jenkins-apn · 2023-07-05T08:57:38Z

daint

🟢 unit test

	Test
🟢	summary

🟢 integration test

	Test
🟢	icon-spack_info
🟢	icon-spack_spec
🟢	icon_serialization=create_claw=std-spack_spec

🔴 system test

	Test
🟢	daint_cpu_cce
🔴	daint_cpu_gcc
🟢	daint_cpu_nvhpc
🟢	daint_cpu_nvhpc_out_of_source
🟢	daint_gpu_nvhpc

WARNING: Serial tests did not run for system tests

dominichofer · 2023-07-06T15:20:59Z

daint_cpu_cce and daint_cpu_cce both passed once, for the same commit. We think that the tests are just flaky and it's not within the scope of this PR to fix that.
@jonasjucker Do you agree? Can we merge?

jonasjucker

So far this variant has no effect. There is no logic bound to it.

Is this on purpose?
I cannot make sense of it.

lxavier · 2023-07-10T07:54:11Z

@jonasjucker this is my request: in the current mast the "gpu" build is named "cuda" which is specific to nvidia gpu's and do not reflect the configure script of ICON. Now we have in icon a --enable-gpu=openacc configure option. I need this PR to use it, the corresponding ICON PR is here: https://gitlab.dkrz.de/icon/icon-nwp/-/merge_requests/950
I already reviewed this. The request was not to review but to notify you that we are going to merge although the CI is red on daint. @PanicSheep run it multiple time with out changing the PR and all test succeeded at list once, so we assume this PR is not breaking anything on daint, but there is an issue with the testing. Do you agree ?
I have updated the description

jonasjucker · 2023-07-10T08:51:53Z

Our tests for ICON are not complete yet.
icon-exclaim with dsl builds are still missing.
I need to understand the changes in this PR to adapt the recipes there.

So my question:
How is the gpu-compilation switch on/off in the env in MR: https://gitlab.dkrz.de/icon/icon-nwp/-/merge_requests/950#139ea4d5ba0b7d8948ab82cd264f57550c4ec414

gpu=openacc+cuda does not do it, since no logic bound to this variant.
I suspect that spack evaluates gpu=openacc+cuda as gpu=openacc and +cuda, so you are setting variant cuda unwillingly to True. This could be because variant gpu uses spack-spec-dsl syntax as possible values.

Alternatively it works because we only check for
if '+cuda' in self.spec: in the recipe.

If so this is very blurry: does +cuda now comes from CudaPackage or from the variant gpu?
If the latter is the case we could remove CudaPackage again from the recipe of icon.

lxavier · 2023-07-10T08:52:41Z

Actually I think we need to pass the openacc+cuda to the configure script, this has other side effects which are needed

lxavier · 2023-07-10T20:12:44Z

I made a try to set --enable-gpu=openacc+gpu

jonasjucker · 2023-07-11T06:50:01Z

launch jenkins icon

jenkins-apn · 2023-07-11T06:54:15Z

tsa

🟢 unit test

	Test
🟢	summary

🟢 integration test

	Test
🟢	icon-spack_info
🟢	icon-spack_spec
🟢	icon_serialization=create_claw=std-spack_spec

jenkins-apn · 2023-07-11T07:04:26Z

balfrin

🟢 unit test

	Test
🟢	summary

🟢 integration test

	Test
🟢	icon-spack_info
🟢	icon-spack_spec
🟢	icon_serialization=create_claw=std-spack_spec

🟢 system test

	Test
🟢	Icon-install_nwp_cpu
🟢	Icon-install_nwp_gpu

jenkins-apn · 2023-07-11T07:51:42Z

daint

🟢 unit test

	Test
🟢	summary

🟢 integration test

	Test
🟢	icon-spack_info
🟢	icon-spack_spec
🟢	icon_serialization=create_claw=std-spack_spec

🟢 system test

	Test
🟢	daint_cpu_cce
🟢	daint_cpu_gcc
🟢	daint_cpu_nvhpc
🟢	daint_cpu_nvhpc_out_of_source
🟢	daint_gpu_nvhpc

jonasjucker · 2023-07-11T08:06:18Z

LGTM.
Is compatible with existing spack.yaml of ICON

repos/c2sm/packages/icon/package.py

dominichofer · 2023-07-11T09:30:00Z

launch jenkins icon

jenkins-apn · 2023-07-11T09:33:16Z

tsa

🟢 unit test

	Test
🟢	summary

🟢 integration test

	Test
🟢	icon-spack_info
🟢	icon-spack_spec
🟢	icon_serialization=create_claw=std-spack_spec

jenkins-apn · 2023-07-11T09:44:16Z

balfrin

🟢 unit test

	Test
🟢	summary

🟢 integration test

	Test
🟢	icon-spack_info
🟢	icon-spack_spec
🟢	icon_serialization=create_claw=std-spack_spec

🟢 system test

	Test
🟢	Icon-install_nwp_cpu
🟢	Icon-install_nwp_gpu

jenkins-apn · 2023-07-11T10:46:49Z

daint

🟢 unit test

	Test
🟢	summary

🟢 integration test

	Test
🟢	icon-spack_info
🟢	icon-spack_spec
🟢	icon_serialization=create_claw=std-spack_spec

🔴 system test

	Test
🔴	daint_cpu_cce
🔴	daint_cpu_gcc
🟢	daint_cpu_nvhpc
🟢	daint_cpu_nvhpc_out_of_source
🟢	daint_gpu_nvhpc

WARNING: Serial tests did not run for system tests

lxavier · 2023-07-11T12:14:57Z

I still need to run the test with ICON: https://gitlab.dkrz.de/icon/icon-nwp/-/merge_requests/950 before this can be merged

lxavier · 2023-07-12T09:57:10Z

@PanicSheep the test is running here https://gitlab.dkrz.de/icon/icon-nwp/-/pipelines/39788 , you can merge if green, otherwise I'll need to look at it.

lxavier · 2023-07-13T15:18:20Z

We have a workaround for the issue in the ICON MR, with this the test is green : https://bb-prod8.dkrz.de/#/builders/87/builds/22 you can merge and create the release

[icon] Adds variant gpu

7b3b544

dominichofer changed the title ~~[icon] Adds variant 'gpu'.~~ [icon] Adds variant 'gpu' and dependency on hip Jun 30, 2023

Removes openacc+hip

040c5ed

dominichofer force-pushed the icon_gpu branch from 7ccf6b6 to 040c5ed Compare July 4, 2023 09:32

Addendum

74f4e55

dominichofer force-pushed the icon_gpu branch from f3fbc42 to 74f4e55 Compare July 4, 2023 09:35

GitHub Action: Apply Pep8-formatting

0e59c77

dominichofer changed the title ~~[icon] Adds variant 'gpu' and dependency on hip~~ [icon] Adds variant 'gpu' Jul 4, 2023

lxavier approved these changes Jul 4, 2023

View reviewed changes

dominichofer marked this pull request as ready for review July 6, 2023 15:17

dominichofer requested a review from jonasjucker July 6, 2023 15:21

jonasjucker requested changes Jul 10, 2023

View reviewed changes

set --enable-gpu=openacc+cuda in ICON configure

0beacd2

lxavier and others added 3 commits July 10, 2023 17:26

fix typo

5c16a9f

Merge branch 'icon_gpu' of github.com:C2SM/spack-c2sm into icon_gpu

2cf1d91

GitHub Action: Apply Pep8-formatting

08d5b84

lxavier and others added 3 commits July 10, 2023 23:42

uses variant gpu=openacc+cuda

6c77c48

Merge branch 'icon_gpu' of github.com:C2SM/spack-c2sm into icon_gpu

511dd15

GitHub Action: Apply Pep8-formatting

51511ea

jonasjucker self-requested a review July 11, 2023 08:05

jonasjucker approved these changes Jul 11, 2023

View reviewed changes

dominichofer commented Jul 11, 2023

View reviewed changes

repos/c2sm/packages/icon/package.py Outdated Show resolved Hide resolved

dominichofer commented Jul 11, 2023

View reviewed changes

repos/c2sm/packages/icon/package.py Outdated Show resolved Hide resolved

Dominic Hofer added 2 commits July 11, 2023 11:29

Update repos/c2sm/packages/icon/package.py

b2d7ae8

Update repos/c2sm/packages/icon/package.py

c386cc7

C2SM deleted a comment from jenkins-apn Jul 11, 2023

dominichofer merged commit 5c54ccd into main Jul 13, 2023

dominichofer deleted the icon_gpu branch July 13, 2023 15:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[icon] Adds variant 'gpu' #782

[icon] Adds variant 'gpu' #782

dominichofer commented Jun 30, 2023 •

edited by lxavier

Loading

github-actions bot commented Jun 30, 2023 •

edited

Loading

lxavier commented Jul 4, 2023

dominichofer commented Jul 4, 2023

lxavier commented Jul 4, 2023

dominichofer commented Jul 4, 2023

dominichofer commented Jul 4, 2023

jenkins-apn commented Jul 4, 2023

jenkins-apn commented Jul 4, 2023

jenkins-apn commented Jul 4, 2023

dominichofer commented Jul 5, 2023

jenkins-apn commented Jul 5, 2023

jenkins-apn commented Jul 5, 2023

jenkins-apn commented Jul 5, 2023

dominichofer commented Jul 6, 2023

jonasjucker left a comment

lxavier commented Jul 10, 2023 •

edited

Loading

jonasjucker commented Jul 10, 2023

lxavier commented Jul 10, 2023 •

edited

Loading

lxavier commented Jul 10, 2023 •

edited

Loading

jonasjucker commented Jul 11, 2023

jenkins-apn commented Jul 11, 2023

jenkins-apn commented Jul 11, 2023

jenkins-apn commented Jul 11, 2023

jonasjucker commented Jul 11, 2023

dominichofer commented Jul 11, 2023

jenkins-apn commented Jul 11, 2023

jenkins-apn commented Jul 11, 2023

jenkins-apn commented Jul 11, 2023

lxavier commented Jul 11, 2023

lxavier commented Jul 12, 2023

lxavier commented Jul 13, 2023

[icon] Adds variant 'gpu' #782

[icon] Adds variant 'gpu' #782

Conversation

dominichofer commented Jun 30, 2023 • edited by lxavier Loading

github-actions bot commented Jun 30, 2023 • edited Loading

lxavier commented Jul 4, 2023

dominichofer commented Jul 4, 2023

lxavier commented Jul 4, 2023

dominichofer commented Jul 4, 2023

dominichofer commented Jul 4, 2023

jenkins-apn commented Jul 4, 2023

tsa

jenkins-apn commented Jul 4, 2023

balfrin

jenkins-apn commented Jul 4, 2023

daint

dominichofer commented Jul 5, 2023

jenkins-apn commented Jul 5, 2023

tsa

jenkins-apn commented Jul 5, 2023

balfrin

jenkins-apn commented Jul 5, 2023

daint

dominichofer commented Jul 6, 2023

jonasjucker left a comment

Choose a reason for hiding this comment

lxavier commented Jul 10, 2023 • edited Loading

jonasjucker commented Jul 10, 2023

lxavier commented Jul 10, 2023 • edited Loading

lxavier commented Jul 10, 2023 • edited Loading

jonasjucker commented Jul 11, 2023

jenkins-apn commented Jul 11, 2023

tsa

jenkins-apn commented Jul 11, 2023

balfrin

jenkins-apn commented Jul 11, 2023

daint

jonasjucker commented Jul 11, 2023

dominichofer commented Jul 11, 2023

jenkins-apn commented Jul 11, 2023

tsa

jenkins-apn commented Jul 11, 2023

balfrin

jenkins-apn commented Jul 11, 2023

daint

lxavier commented Jul 11, 2023

lxavier commented Jul 12, 2023

lxavier commented Jul 13, 2023

dominichofer commented Jun 30, 2023 •

edited by lxavier

Loading

github-actions bot commented Jun 30, 2023 •

edited

Loading

lxavier commented Jul 10, 2023 •

edited

Loading

lxavier commented Jul 10, 2023 •

edited

Loading

lxavier commented Jul 10, 2023 •

edited

Loading