Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/nix support #131

Merged
merged 8 commits into from
Aug 1, 2024
Merged

Feature/nix support #131

merged 8 commits into from
Aug 1, 2024

Conversation

jake-arkinstall
Copy link
Collaborator

Description

This is a WIP PR to add nix support to the repo. The aim is to combat the packaging requirements around cuda and tket.

It is not ready for merging yet. For effective use there are three things remaining:

  • Documentation
  • Remote Caching with cachix. At current, cachix does not skip uploading packages that are already available in the upstream cuda-maintainers.cachix.org cache, so a push would be quite large and fill up precious space. I have reached out to Domen and he has told me that an update to help us on this will be out next week.
  • Usage with non-nixos systems. I have successfully run this flake on an ubuntu docker image with cuda drivers loaded, but it is awkward. We need to make use of a third-party NixGL flake and wrap binaries in it, in order to have the system's cuda drivers available for nix to use. I want to improve this somewhat, otherwise it will be confusing for users.

When this is ready and merged, CI should build any changes and upload them to cachix. I will pre-populate cachix with the heavy dependencies to make this process quick and easy. When this is all in place, we can think about use the nix flake for testing in CI using a gpu-enabled machine without being concerned about the awkward dependency setup process.

Related issues

N/A

Checklist

  • I have run the tests on a device with GPUs.
  • I have performed a self-review of my code.
  • I have commented hard-to-understand parts of my code.
  • I have made corresponding changes to the public API documentation.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have updated the changelog with any user-facing changes.

@jake-arkinstall
Copy link
Collaborator Author

This is still waiting on two things:

  1. Cachix environment variable - when set, built binaries will be uploaded to the cache. However, we do not wish to do this until Cachix supports adding additional upstreams, otherwise we end up uploading bulky nvidia libraries to our own cache, and we have limited storage. If we could add cuda-maintainers.cachix.org to our upstreams, this would not happen. After reaching out to Domen, he mentioned that this functionality is on its way.

  2. A test runner. We need GPU capabilities on a github actions runner in order for this to work. I am happy to donate my dev machine for this but we could do with something in-office.

Additionally, I am now leaning towards the idea of centralising our nix stuff into its own 'tketpkgs' repo.

This commit utilises some cuda and python packages from nixpkgs
to fetch dependencies, and also provides missing dependencies
and upgrades to several dependencies. It allows one to run
`nix build .#pytket-cutensornet` (or `nix build` for short) to build
the tket extension.

Tests can't be run within the normal `nix flake check` context
as the GPU drivers aren't available within the sandbox, so tests
can be run with `nix run .#tests` instead.

`nix develop` within the source root can be used to enter a shell
with pytket's cutensornet backend available. If and when this commit
is merged into main, users will be able to run:

`nix develop github:cqcl/pytket-cutensornet`

and have the environment fully loaded for their use.

Cuda support is verified on nixos. Verification on non-nixos nix
installations will be required for general use. Integration with
Cachix will be extremely beneficial due to the size and duration
of the builds.
@jake-arkinstall jake-arkinstall marked this pull request as ready for review July 31, 2024 12:55
Copy link
Collaborator

@PabloAndresCQ PabloAndresCQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a chat with Jake, makes sense!

@jake-arkinstall jake-arkinstall merged commit 1c5d293 into main Aug 1, 2024
8 checks passed
@PabloAndresCQ PabloAndresCQ deleted the feature/nix-support branch August 1, 2024 10:07
@PabloAndresCQ PabloAndresCQ restored the feature/nix-support branch August 1, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants