Skip to content

Conversation

@li-roy
Copy link
Contributor

@li-roy li-roy commented Jun 19, 2019

Stack from ghstack:

Remove Type dispatch and related things that aren't needed anymore. The derived type classes still exist for now, but basically act just as namespaces.

Removed:

  • Type class hierarchy
  • most of LegacyTypeDispatch
  • ComplexHooksInterface
  • Type registration apis
  • Parts of CUDA and CPU hooks related to Type registration
  • LegacyTHDispatcher
  • getter apis for Types, e.g. getVariableType

Differential Revision: D15897424

@pytorchbot pytorchbot added module: cuda Related to torch.cuda, and CUDA support in general module: internals Related to internal abstractions in c10 and ATen labels Jun 19, 2019
@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Jun 19, 2019
royboy added 6 commits June 19, 2019 10:46
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
virtual const char * toString() const override;
virtual TypeID ID() const override;

struct CAFFE2_API ${Type} {
Copy link
Contributor

Choose a reason for hiding this comment

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

we still need this?

Copy link
Contributor

Choose a reason for hiding this comment

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

did you answer this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Going to get rid of it.

Copy link
Contributor

@gchanan gchanan left a comment

Choose a reason for hiding this comment

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

looks like there's an XLA problem, but I'm guessing you are working through that already.

royboy added 8 commits June 21, 2019 15:27
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
@ezyang ezyang removed their request for review June 24, 2019 13:35
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
@li-roy li-roy mentioned this pull request Jun 24, 2019
royboy added 9 commits June 24, 2019 12:54
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
#pragma once

// ${generated_comment}
#include <ATen/ATen.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we still need TypeDefault? Is it because you don't want to break extension mechanisms or you still want a place to have all the dispatch code? (i.e. done want to inline them in the methods or whatever?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the Type classes are basically just acting as namespaces for now. TypeDefault is for the wrappers and registrations for non backend specific native functions.

Copy link
Contributor

@gchanan gchanan left a comment

Choose a reason for hiding this comment

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

lgtm, I have a couple of outstanding questions.

Remove Type dispatch

gh-metadata: pytorch pytorch 21964 gh/li-roy/32/head
@zou3519 zou3519 deleted the gh/li-roy/32/head branch June 30, 2019 11:14
@facebook-github-bot
Copy link
Contributor

@li-roy merged this pull request in 2a69868.

zdevito pushed a commit to zdevito/ATen that referenced this pull request Jun 30, 2019
Summary:
Pull Request resolved: pytorch/pytorch#21964
ghimport-source-id: fdfb555ac4efbf31ae7d2c700a5aa44ad0cc4d7f

Test Plan: Imported from OSS

Differential Revision: D15897424

Pulled By: li-roy

fbshipit-source-id: 3cd6744254e34d70e6875ffde749b5cf959b663c
xzhu1900 pushed a commit to xzhu1900/pytorch that referenced this pull request Jul 5, 2019
Summary:
Pull Request resolved: pytorch#21964
ghimport-source-id: fdfb555

Test Plan: Imported from OSS

Differential Revision: D15897424

Pulled By: li-roy

fbshipit-source-id: 3cd6744254e34d70e6875ffde749b5cf959b663c
@dylanbespalko dylanbespalko mentioned this pull request Jul 8, 2019
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Merged module: cuda Related to torch.cuda, and CUDA support in general module: internals Related to internal abstractions in c10 and ATen oncall: jit Add this issue/PR to JIT oncall triage queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants