Skip to content

Add quirks.py for GPU/platform-specific quirks; disable CuDNN benchmarking on ROCm#2558

Merged
asumagic merged 6 commits intospeechbrain:developfrom
asumagic:gpu-quirks
Oct 22, 2024
Merged

Add quirks.py for GPU/platform-specific quirks; disable CuDNN benchmarking on ROCm#2558
asumagic merged 6 commits intospeechbrain:developfrom
asumagic:gpu-quirks

Conversation

@asumagic
Copy link
Collaborator

@asumagic asumagic commented May 30, 2024 •
edited
Loading

What does this PR do?

Fixes #2552

The code should be fairly well documented. The idea is that SpeechBrain now would take a more structured/documented approach to overriding PyTorch's global configuration.

So-called "quirks" represent an individual configuration change, and it may be unconditional or depend on hardware. The user is notified by an info log when quirks are applied.

This adds a quirk to disable CuDNN benchmarking on ROCm as it degrades performance for a majority of cases in SpeechBrain.

Quirks can be explicitly opted out from by setting an environment variable, SB_DISABLE_QUIRKS.

Example:

$ SB_DISABLE_QUIRKS=disable_cudnn_benchmarking python3
Python 3.9.18 (main, Sep 11 2023, 13:41:44) 
[GCC 11.2.0] :: Anaconda, Inc. on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import logging; logging.basicConfig(level=logging.INFO); import speechbrain
INFO:speechbrain.utils.quirks:Applied quirks (see `speechbrain.utils.quirks`): [disable_jit_profiling, allow_tf32]
INFO:speechbrain.utils.quirks:Excluded quirks specified by the `SB_DISABLE_QUIRKS` environment (comma-separated list): [disable_cudnn_benchmarking]
Before submitting
  • Did you read the contributor guideline?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you make sure to update the documentation with your changes? (if necessary)
  • Did you write any new necessary tests? (not for typos and docs)
  • Did you verify new and existing tests pass locally with your changes?
  • Did you list all the breaking changes introduced by this pull request?
  • Does your code adhere to project-specific code style and conventions?

PR review

Reviewer checklist
  • Is this pull request ready for review? (if not, please submit in draft mode)
  • Check that all items from Before submitting are resolved
  • Make sure the title is self-explanatory and the description concisely explains the PR
  • Add labels and milestones (and optionally projects) to the PR so it can be classified
  • Confirm that the changes adhere to compatibility requirements (e.g., Python version, platform)
  • Review the self-review checklist to ensure the code is ready for review

@mravanelli mravanelli requested a review from Adel-Moumen June 17, 2024 16:23
@mravanelli mravanelli added the enhancement New feature or request label Jun 17, 2024
@asumagic asumagic added the performance Issue or fix for something slow or memory-hungry label Jul 18, 2024
@asumagic asumagic marked this pull request as ready for review September 11, 2024 07:39
@asumagic
Copy link
Collaborator Author

asumagic commented Sep 17, 2024

BTW heads up @Adel-Moumen since this is moving the allow_tf32 code. Nothing should be functionally different by default, though.

@asumagic asumagic added this to the v1.1.0 milestone Sep 17, 2024
@Adel-Moumen
Copy link
Collaborator

Adel-Moumen commented Oct 18, 2024

LGTM. Before merging, you need to add a proper documentation as it's seems to be something that the user should be aware of.

@asumagic
Copy link
Collaborator Author

asumagic commented Oct 18, 2024

LGTM. Before merging, you need to add a proper documentation as it's seems to be something that the user should be aware of.

Where do you suggest that documentation to be/what do you suggest that it covers?
I'd agree (though I think the file is fairly self-descriptive on its own), but I'm not sure where to put it in a way that's easily discovered.

@Adel-Moumen
Copy link
Collaborator

Adel-Moumen commented Oct 18, 2024

In docs/experiment.md I would say

@asumagic asumagic merged commit 1bb368a into speechbrain:develop Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Adel-Moumen Adel-Moumen Awaiting requested review from Adel-Moumen

Assignees

@asumagic asumagic

Labels

enhancement New feature or request performance Issue or fix for something slow or memory-hungry

Projects

None yet

Milestone

v1.0.2

Development

Successfully merging this pull request may close these issues.

AMD ROCm: torch.backends.cudnn.benchmark should be set to False by default on ROCm

3 participants

@asumagic @Adel-Moumen @mravanelli