migrating deprecated calls without abc module for containers #11515
Conversation
jeffreyksmithjr
requested review from
anderspapitto,
apaszke,
bddppq,
colesbury,
dzhulgakov,
ezyang,
gchanan,
houseroad,
jamesr66a,
smessmer,
soumith,
Yangqing and
zdevito
as code owners
pytorchbot
added
caffe2
oncall: jit
|
The reason why we used those is that |
There was a problem hiding this comment.
jeffreyksmithjr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
jeffreyksmithjr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
jeffreyksmithjr
force-pushed
the
master
branch
from
0704613
to
b33b594
Compare
There was a problem hiding this comment.
jeffreyksmithjr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
jeffreyksmithjr
force-pushed
the
master
branch
from
b33b594
to
b237ef7
Compare
There was a problem hiding this comment.
jeffreyksmithjr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
jeffreyksmithjr
force-pushed
the
master
branch
from
b237ef7
to
02bc339
Compare
There was a problem hiding this comment.
jeffreyksmithjr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@pytorchbot retest this please |
|
@pytorchbot retest this please |
|
The Circle tests all flaked (but I think that's a more global issue). The ROCM tests failed, which is also a known issue. With @ssnl 's LGTM do we call this mergeable or are we going to try to get any of those failed builds to pass? |
|
@jeffreyksmithjr one of the builds has been broken for at least a day, another one has timed out, but it's unlikely that it failed. CircleCI is experimental, so you can ignore any failures for now. Go ahead and merge this |
|
(Although it might be good to get a review of the C2 bits from someone working on C2) |
|
Makes sense. There is some tiny amount of Caffe2 impact. Could any Caffe2 experts review this PR? |
jeffreyksmithjr
force-pushed
the
master
branch
from
02bc339
to
e18207f
Compare
There was a problem hiding this comment.
jeffreyksmithjr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@Yangqing If we do, it could be as simple as |
|
Paraphrasing what @apaszke told me, the reason for Clearly, the current approach does create a bit of obvious redundancy, though. |
jeffreyksmithjr
force-pushed
the
master
branch
from
e18207f
to
79e7409
Compare
There was a problem hiding this comment.
jeffreyksmithjr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
jeffreyksmithjr
force-pushed
the
master
branch
from
79e7409
to
abdf04b
Compare
There was a problem hiding this comment.
jeffreyksmithjr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
jeffreyksmithjr is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
jeffreyksmithjr is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Implementing #10540.