ginkgo-hpc: init at 1.6.0 #261155

Madouura · 2023-10-15T05:31:41Z

Description of changes

Tracking: #197885
For: #216655
@keldu I added you to the maintainers for ginkgo-hpc since you expressed interest in making a PR for this yourself.

Things done

markuskowa · 2023-10-15T11:24:46Z

Thanks for working on this.
Since this is in draft stage, I leave some general comments here to help with future iterations:

New packages should go into /pkgs/by-name/... (see https://github.com/NixOS/rfcs/blob/master/rfcs/0140-simple-package-paths.md)
Please use lib.getDev instead of pkname.dev if you need to find headers.
Instead of making the build of docs, examples, etc. optional, I would recommend to rather use multiple outputs.
Please consider to split up the PR into several PRs. That makes it easier to review.
What is pmix-5 needed for at the moment ? I would rather go with having only version (if possible). OpenMPI is only compatible with pmix-3 (if I remember correctly). PMix is tightly coupled to MPI and Slurm. Thus openmpi and pmix should be updated together once openmpi-5 is released.

Madouura · 2023-10-15T12:02:26Z

Thanks for working on this. Since this is in draft stage, I leave some general comments here to help with future iterations:

* New packages should go into `/pkgs/by-name/...` (see https://github.com/NixOS/rfcs/blob/master/rfcs/0140-simple-package-paths.md)

* Please use `lib.getDev` instead of `pkname.dev` if you need to find headers.

* Instead of making the build of docs, examples, etc. optional, I would recommend to rather use  multiple outputs.

* Please consider to split up the PR into several PRs. That makes it easier to review.

* What is `pmix-5` needed for at the moment ? I would rather go with having only version (if possible). OpenMPI is only compatible with pmix-3 (if I remember correctly). PMix is tightly coupled to MPI and Slurm. Thus openmpi and pmix should be updated together once openmpi-5 is released.

pmix_5 is needed because openmpi-rocm overrides openmpi to use the 5.0.x branch, which is required for working ROCm support. We can remove most of this override as well as pmix_5 once openmpi 5.0 is released.
To clarify a bit further; openmpi 5.0 requires pmix 5.0

keldu · 2023-10-15T12:07:19Z

Gladly taking the maintainer role here.
My derivation file used in the issue #216655 regarding hip was more meant to debug the issue with hip I kept having in nixos-23.05 with the future intent of doing this PR. I guess it got fast tracked a little bit.

Thanks for your work.

Madouura · 2023-10-15T18:59:57Z

That should fix most of what you asked, @markuskowa.
As for the multiple PRs thing, normally I would but everything here's pretty tightly integrated with maybe the exception of kokkos only being needed for making examples.

markuskowa · 2023-10-15T21:51:09Z

To clarify a bit further; openmpi 5.0 requires pmix 5.0

I have been preparing openmpi-5.0 (https://github.com/markuskowa/nixpkgs/tree/openmpi-5.0), so once it is released, were are actually ready to update.

As for the multiple PRs thing, normally I would but everything here's pretty tightly integrated with maybe the exception of kokkos only being needed for making examples.

The additions of rocm to ucc, ucx, and openmpi could be its own PR first, for example.

Madouura · 2023-10-16T02:58:52Z

To clarify a bit further; openmpi 5.0 requires pmix 5.0

I have been preparing openmpi-5.0 (https://github.com/markuskowa/nixpkgs/tree/openmpi-5.0), so once it is released, were are actually ready to update.

As for the multiple PRs thing, normally I would but everything here's pretty tightly integrated with maybe the exception of kokkos only being needed for making examples.

The additions of rocm to ucc, ucx, and openmpi could be its own PR first, for example.

Looking through your commits, I think I see why pmix works on your branch. It's at 4.0, which apparently is good enough for openmpi 5.0. I still see no reason to get rid of pmix_5, but that's good to know.

The additions of rocm to ucc, ucx, and openmpi could be its own PR first, for example.

This is indev right now anyway, so sure alright.

Madouura · 2023-10-16T03:04:13Z

Keeping the commits here until those two PRs are merged so I can continue to build and test.

Move pmix to pmix_3, and make pmix_3 the default

Madouura · 2023-10-16T05:36:21Z

I'm using this package (ginkgo-hpc) as a bit of a testing bed for rocmPackages changes, including impureTests.
If you have any further nitpicks at all, let me know. The changes here are going to be reflected across the entire package set.

Artturin · 2023-10-17T06:23:02Z

pkgs/by-name/gi/ginkgo-hpc/package.nix

+ cuda = finalAttrs.finalPackage.overrideAttrs (callPackage ./overlays/cuda.nix { });
+ hip = finalAttrs.finalPackage.overrideAttrs (callPackage ./overlays/hip.nix { inherit rocmUnfree; });
+ dpc = finalAttrs.finalPackage.overrideAttrs (callPackage ./overlays/dpc.nix { });
+ doc = finalAttrs.finalPackage.overrideAttrs (callPackage ./overlays/doc.nix { });
+ test = finalAttrs.finalPackage.overrideAttrs (callPackage ./overlays/test.nix { });
+ example = finalAttrs.finalPackage.overrideAttrs (callPackage ./overlays/example.nix { });
+ benchmark = finalAttrs.finalPackage.overrideAttrs (callPackage ./overlays/benchmark.nix { });


the derivations in passthru won't be spliced because
#211340

so having one of them in nativeBuildInputs when cross-compiling won't pick the on build version

nix-repl> pkgsCross.aarch64-multiplatform.__splicedPackages.ginkgo-hpc.doc ? __spliced false

So, unless I'm reading this completely wrong...

I need to make a scope like https://github.com/NixOS/nixpkgs/blob/master/pkgs/top-level/cuda-packages.nix

I need to use makeScopeWithSplicing somehow?

If those two options won't work, would just setting it to x86-64-linux for now suffice?
I don't really understand this at all.

Artturin · 2023-10-17T06:23:46Z

pkgs/by-name/gi/ginkgo-hpc/package.nix

+ hip = finalAttrs.finalPackage.overrideAttrs (callPackage ./overlays/hip.nix { inherit rocmUnfree; });
+ dpc = finalAttrs.finalPackage.overrideAttrs (callPackage ./overlays/dpc.nix { });
+ doc = finalAttrs.finalPackage.overrideAttrs (callPackage ./overlays/doc.nix { });
+ test = finalAttrs.finalPackage.overrideAttrs (callPackage ./overlays/test.nix { });


tests should be in passthru.tests attrset

The test sub-derivation just builds the tests, we have to use impureTests for actually running them.
This is because said tests require a GPU.

This was referenced Oct 15, 2023

ROCm gfx10 and gfx11 not supported ginkgo-project/ginkgo#1429

Open

[Tracking] ROCm packages #197885

Open

ofborg bot added the 8.has: package (new) label Oct 15, 2023

ofborg bot requested a review from markuskowa October 15, 2023 05:50

ofborg bot added 10.rebuild-darwin: 101-500 10.rebuild-linux: 101-500 labels Oct 15, 2023

Madouura force-pushed the pr/ginkgo branch from 93464fe to e19daca Compare October 15, 2023 11:49

Madouura force-pushed the pr/ginkgo branch 6 times, most recently from 4793e9f to 8c5ee98 Compare October 15, 2023 18:58

Madouura force-pushed the pr/ginkgo branch 2 times, most recently from fa7dd6b to 4695722 Compare October 15, 2023 19:36

This was referenced Oct 16, 2023

pmix_5: init at 5.0.1, {ucx,ucc,openmpi}: add rocm support #261315

Draft

kokkos: init at 4.1.00 #261316

Merged

Madouura force-pushed the pr/ginkgo branch from 4695722 to 1f7dff6 Compare October 16, 2023 05:28

Madouura added 5 commits October 16, 2023 00:30

pmix_5: init at 5.0.1

67a2e00

Move pmix to pmix_3, and make pmix_3 the default

ucx: add rocm support

bbd6e34

ucc: add rocm support

fe1fb0a

openmpi: add rocm support

8b0ccc7

kokkos: init at 4.1.00

9426c44

Madouura force-pushed the pr/ginkgo branch from 1f7dff6 to 5d9824f Compare October 16, 2023 05:37

ginkgo-hpc: init at 1.6.0

f81dcaa

Madouura force-pushed the pr/ginkgo branch from 5d9824f to f81dcaa Compare October 16, 2023 08:10

Artturin reviewed Oct 17, 2023

View reviewed changes

wegank added the 2.status: merge conflict label Mar 20, 2024

wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ginkgo-hpc: init at 1.6.0 #261155

ginkgo-hpc: init at 1.6.0 #261155

Madouura commented Oct 15, 2023 •

edited

Loading

markuskowa commented Oct 15, 2023

Madouura commented Oct 15, 2023 •

edited

Loading

keldu commented Oct 15, 2023 •

edited

Loading

Madouura commented Oct 15, 2023

markuskowa commented Oct 15, 2023

Madouura commented Oct 16, 2023 •

edited

Loading

Madouura commented Oct 16, 2023

Madouura commented Oct 16, 2023

Artturin Oct 17, 2023 •

edited

Loading

Madouura Oct 17, 2023

Madouura Oct 17, 2023

Artturin Oct 17, 2023

Madouura Oct 17, 2023 •

edited

Loading

ginkgo-hpc: init at 1.6.0 #261155

Are you sure you want to change the base?

ginkgo-hpc: init at 1.6.0 #261155

Conversation

Madouura commented Oct 15, 2023 • edited Loading

Description of changes

Things done

markuskowa commented Oct 15, 2023

Madouura commented Oct 15, 2023 • edited Loading

keldu commented Oct 15, 2023 • edited Loading

Madouura commented Oct 15, 2023

markuskowa commented Oct 15, 2023

Madouura commented Oct 16, 2023 • edited Loading

Madouura commented Oct 16, 2023

Madouura commented Oct 16, 2023

Artturin Oct 17, 2023 • edited Loading

Choose a reason for hiding this comment

Madouura Oct 17, 2023

Choose a reason for hiding this comment

Madouura Oct 17, 2023

Choose a reason for hiding this comment

Artturin Oct 17, 2023

Choose a reason for hiding this comment

Madouura Oct 17, 2023 • edited Loading

Choose a reason for hiding this comment

Madouura commented Oct 15, 2023 •

edited

Loading

Madouura commented Oct 15, 2023 •

edited

Loading

keldu commented Oct 15, 2023 •

edited

Loading

Madouura commented Oct 16, 2023 •

edited

Loading

Artturin Oct 17, 2023 •

edited

Loading

Madouura Oct 17, 2023 •

edited

Loading