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.

Sign up for GitHub

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

Jump to bottom

rocfft: split base outputs for hydra #214606

Merged
merged 2 commits into from
Apr 1, 2023
Merged

rocfft: split base outputs for hydra #214606

merged 2 commits into from
Apr 1, 2023

Conversation

Madouura
Copy link
Contributor

@Madouura Madouura commented Feb 4, 2023 •
edited
Loading

Description of changes

Tracking: #197885
Last thing I can do before giving up on trying to get this to cache on hydra.
Technically I can have rocfft build twice, but then we'd have to do the same for composable_kernel, which I've learned is just barely under the output limit.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@Madouura
Copy link
Contributor Author

Madouura commented Feb 4, 2023 •
edited
Loading

It built hipfft and I'm not rebuilding torch again, so consider nixpkgs-review checked please.
Ended up having to build torch for different reasons, upcoming post will cover nixpkgs-review.

@Madouura Madouura mentioned this pull request Feb 4, 2023
Open
34 tasks
@ofborg ofborg bot requested a review from Flakebi February 4, 2023 21:28
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 labels Feb 4, 2023
@Madouura
rocfft: fix propagatedBuildInputs typo
9cb1ca6 
propagatedBuildInputs has been incorrectly spelled for a while now.
@Madouura
rocfft: split base outputs for hydra
4af7dad 
Split the base derivation's outputs to hopefully avoid hydra failing the base due to the output limit and not continuing with the rest.
This and all hydra commits for rocfft and composable_kernel will be reverted if this does not work.
@Madouura
Copy link
Contributor Author

Madouura commented Feb 8, 2023

ofborg failing seems to be nixpkgs-wide at the moment and does not seem to be related to this PR.
Having ofborg redo the checks should fix this once the root problem is fixed.

@Madouura
Copy link
Contributor Author

Madouura commented Feb 8, 2023

Result of nixpkgs-review pr 214606 run on x86_64-linux 1

3 packages built:
  • hipfft
  • python310Packages.torchWithRocm
  • rocfft

Flakebi
Flakebi approved these changes Mar 1, 2023
View reviewed changes
Copy link
Member

@Flakebi Flakebi left a comment

Choose a reason for hiding this comment

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

Do the intermediate rf-zero..rf-three derivations have an purpose left?

Anyway, LGTM, it’s worth a try.

@Madouura
Copy link
Contributor Author

Madouura commented Mar 4, 2023

I honestly don't know, not getting much help from those with more know-how on the matter so this was more or less the last spaghetti noodle I could throw at the wall and see if it stuck.

@figsoda figsoda added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Mar 5, 2023
@gbtb gbtb mentioned this pull request Mar 11, 2023
Closed
5 tasks
@SuperSandro2000
Copy link
Member

@ofborg eval

@ofborg ofborg bot requested a review from Flakebi April 1, 2023 00:58
@SuperSandro2000 SuperSandro2000 merged commit 50b19b0 into NixOS:master Apr 1, 2023
@Madouura Madouura deleted the pr/rocfft branch April 1, 2023 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@Flakebi Flakebi Awaiting requested review from Flakebi

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Madouura @SuperSandro2000 @Flakebi @figsoda