Skip to content

#26426 fixed#28715

Closed
drnikolaev wants to merge 60 commits intopytorch:masterfrom
drnikolaev:pr_trt6
Closed

#26426 fixed#28715
drnikolaev wants to merge 60 commits intopytorch:masterfrom
drnikolaev:pr_trt6

Conversation

@drnikolaev
Copy link
Contributor

This is the fix for reverted #26426
@houseroad @bddppq @soumith

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Member

@houseroad houseroad left a comment

Choose a reason for hiding this comment

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

I will create another PR to test the code, if it is okay, i will merge this one.

@drnikolaev
Copy link
Contributor Author

drnikolaev commented Oct 25, 2019

sure :)

@houseroad
Copy link
Member

Some CI won't be trigger here, so I have to explicitly trigger it.

This was referenced Oct 25, 2019
@houseroad
Copy link
Member

The build still fails. Check #28720

@houseroad
Copy link
Member

@drnikolaev
Copy link
Contributor Author

@houseroad I didn't know that you keep TRT5 permanently installed on your CI boxes. It's incompatible with TRT6. Is this OK to return TRT6 installer back to my PR? Just installer, no CI integration.

@houseroad
Copy link
Member

@drnikolaev do you mean here: https://github.com/pytorch/pytorch/blob/master/.jenkins/caffe2/build.sh#L133

If so, feel free to update it.

@drnikolaev
Copy link
Contributor Author

drnikolaev commented Oct 25, 2019

@houseroad Yes but let's keep it simple. New parser needs TRT6, so why bother? If no TRT6 installed, turn the feature off and issue a warning.
That installer used to serve CI integration only.

@drnikolaev
Copy link
Contributor Author

@houseroad I tried to clone your branch here: #28737 but still can't see caffe2_py2_cuda9_0_cudnn7_ubuntu16_04 target. Other ones look good.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@houseroad
Copy link
Member

Seems good. Let me do one more check.

zdevito pushed a commit to zdevito/ATen that referenced this pull request Nov 1, 2019
Summary:
This is the fix for reverted pytorch/pytorch#26426
houseroad bddppq soumith
Pull Request resolved: pytorch/pytorch#28715

Reviewed By: hl475

Differential Revision: D18146731

Pulled By: houseroad

fbshipit-source-id: 247366451a6334e84df82d00339521f797b33130
@facebook-github-bot
Copy link
Contributor

@houseroad merged this pull request in 1e2049c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants