Skip to content

python3Packages.torch: Suppress spammy warnings#266995

Merged
Artturin merged 1 commit intoNixOS:staging-nextfrom
Madouura:pr/fix-torch-log-limit
Nov 14, 2023
Merged

python3Packages.torch: Suppress spammy warnings#266995
Artturin merged 1 commit intoNixOS:staging-nextfrom
Madouura:pr/fix-torch-log-limit

Conversation

@Madouura
Copy link
Copy Markdown
Contributor

@Madouura Madouura commented Nov 12, 2023 •
edited
Loading

Description of changes

Fixes #266895
Before it is asked, we cannot just simply define the HIP_CXX_FLAGS variable in preConfigure.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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.11 Release Notes (or backporting 23.05 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
  • Fits CONTRIBUTING.md.

@github-actions github-actions Bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Nov 12, 2023
@ofborg ofborg Bot requested review from teh, thoughtpolice and tscholak November 12, 2023 08:03
@ofborg ofborg Bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Nov 12, 2023
Artturin
Artturin reviewed Nov 12, 2023
View reviewed changes
Comment thread pkgs/development/python-modules/torch/default.nix Outdated
@Madouura Madouura force-pushed the pr/fix-torch-log-limit branch from 5880bf7 to 113a33e Compare November 12, 2023 10:47
@Madouura Madouura changed the title python3Packages.torch: Suppress spammy warnings for ROCm build python3Packages.torch: Suppress spammy warnings Nov 12, 2023
@Madouura
Copy link
Copy Markdown
Contributor Author

Madouura commented Nov 12, 2023 •
edited
Loading

Suppressed the worst of the warnings for regular and ROCm variants.
Not doing any CUDA-specific warnings because it's not cached anyway.
--amdgpu-target warning spam will be fixed by #259068.

@ofborg ofborg Bot added 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Nov 12, 2023
@tpwrules
Copy link
Copy Markdown
Contributor

tpwrules commented Nov 12, 2023

Would be a good idea to add a comment explaining why the flags were added.

@wegank
Copy link
Copy Markdown
Member

wegank commented Nov 12, 2023

Could you please rebase this on staging-next? The flags there are different for Clang support.

@Madouura
Copy link
Copy Markdown
Contributor Author

Madouura commented Nov 13, 2023 •
edited
Loading

Would be a good idea to add a comment explaining why the flags were added.

The comment is in the commit message. @mweinelt keyed me in on why this is better than adding it to the derivation.
Edit: On second thought, since I'm rebasing anyway, I'll just add the comment and maybe someone will tell me to remove it or not, I don't know.

Could you please rebase this on staging-next? The flags there are different for Clang support.

I'll try building it on staging-next locally, but if it works the same I don't think there's a reason to rebase here on the PR?
Edit: Never mind, I thought you meant clang flags in general. Looks like there's a merge conflict so I will do so.

@Madouura Madouura changed the base branch from master to staging-next November 13, 2023 02:27
@Madouura Madouura force-pushed the pr/fix-torch-log-limit branch from 113a33e to f0357ed Compare November 13, 2023 02:30
Artturin
Artturin reviewed Nov 13, 2023
View reviewed changes
Comment thread pkgs/development/python-modules/torch/default.nix Outdated
@Madouura
python3Packages.torch: Suppress spammy warnings
c8f136c 
Suppresses the most spammy warnings.
This is mainly to fix NixOS#266895.
@Madouura Madouura force-pushed the pr/fix-torch-log-limit branch from f0357ed to c8f136c Compare November 14, 2023 04:05
@Artturin Artturin merged commit 2605fad into NixOS:staging-next Nov 14, 2023
@Madouura Madouura deleted the pr/fix-torch-log-limit branch November 14, 2023 10:31
@wegank
Copy link
Copy Markdown
Member

wegank commented Nov 17, 2023

Has anyone tested this on darwin before merging? It seems that some flags are not recognized by Clang in #267209.

@wegank wegank mentioned this pull request Nov 17, 2023
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Artturin Artturin Artturin left review comments

@tscholak tscholak Awaiting requested review from tscholak

@teh teh Awaiting requested review from teh

@thoughtpolice thoughtpolice Awaiting requested review from thoughtpolice

Assignees

No one assigned

Labels

6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Build failure: torchWithRocm: Log limit exceeded

4 participants

@Madouura @tpwrules @wegank @Artturin