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

attempted fix for nvrtc with lovelace #87611

Closed
wants to merge 1 commit into from
Closed

Conversation

ngimel
Copy link
Collaborator

@ngimel ngimel commented Oct 24, 2022

Fixes #87595 (maybe?)

@pytorch-bot
Copy link

pytorch-bot bot commented Oct 24, 2022

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/87611

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 40f84e1:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@pytorch-bot pytorch-bot bot added the release notes: jit release notes category label Oct 24, 2022
@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Oct 24, 2022
@@ -893,6 +893,8 @@ void codegenOutputQuery(
max_dev_version = CUDAVersion(7, 5);
} else if (nvrtc_version == CUDAVersion(11, 0)) { // 11.0 supports 3-8.0
max_dev_version = CUDAVersion(8, 0);
} else if (nvrtc_major == 11 && nvrtc_minor < 8) {
Copy link
Contributor

@malfet malfet Oct 24, 2022

Choose a reason for hiding this comment

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

Suggested change
} else if (nvrtc_major == 11 && nvrtc_minor < 8) {
} else if (nvrtc_major == 11 && nvrtc_minor <= 8) {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

BUt versions less than 11.8 support only up to sm_86, so it's like this on purpose, or is my reasoning wrong?

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought nvrtc from 11.8 does not support sm_89, as well, does it?

Copy link
Collaborator Author

@ngimel ngimel Oct 24, 2022

Choose a reason for hiding this comment

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

It should, according to #87436 and the docs, cc @eqy and @ptrblck to double check. That PR is of course for nvcc, not nvrtc, but I would expect nvrtc support matrix to be similar.

@eqy
Copy link
Collaborator

eqy commented Oct 24, 2022

I believe this should work, as I've tested a similar fix with an older CUDA version previously as well

CC @ptrblck

@ngimel
Copy link
Collaborator Author

ngimel commented Oct 24, 2022

@pytorchbot merge -g

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks on your PR pass since you used the green (-g) flag (ETA: 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@ngimel
Copy link
Collaborator Author

ngimel commented Oct 24, 2022

@pytorchbot merge -f "low risk change that's not tested in CI anyway"

@pytorchmergebot
Copy link
Collaborator

The merge job was canceled. If you believe this is a mistake,then you can re trigger it through pytorch-bot.

@atalman
Copy link
Contributor

atalman commented Oct 24, 2022

@pytorchbot merge -f "low risk change that's not tested in CI anyway"

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@github-actions
Copy link

Hey @ngimel.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

atalman added a commit that referenced this pull request Oct 24, 2022
Fixes #87595 (maybe?)

Pull Request resolved: #87611
Approved by: https://github.com/malfet, https://github.com/atalman

Co-authored-by: Natalia Gimelshein <ngimel@fb.com>
@xsacha
Copy link
Contributor

xsacha commented Oct 25, 2022

Will this be in torch 1.13?

sgrigory pushed a commit to sgrigory/pytorch that referenced this pull request Oct 28, 2022
kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Nov 5, 2022
kulinseth pushed a commit to kulinseth/pytorch that referenced this pull request Dec 10, 2022
@github-actions github-actions bot deleted the ngimel/lovelace_arch branch April 25, 2024 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ciflow/trunk Trigger trunk jobs on your pull request Merged release notes: jit release notes category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nvrtc: error: invalid value for --gpu-architecture (-arch)
6 participants