-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
[Unify Tensors PR #7] Merged LoDTensor with Tensor, test=allcases #38880
[Unify Tensors PR #7] Merged LoDTensor with Tensor, test=allcases #38880
Conversation
Thanks for your contribution! |
064eb19
to
d4f0596
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some comment
@@ -31,7 +31,7 @@ void SetLoD(DstLoD* dst, const SrcLoD& src) { | |||
} | |||
} | |||
|
|||
std::unique_ptr<pten::DenseTensor> MakePtenDenseTensor( | |||
std::unique_ptr<pten::DenseTensor> MakePtenDenseTensorOrig( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bad name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do u mean by Orig
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to MakePtenDenseTensorBase().
Basically, these functions under "/pten/api/" will be removed after Tensor Unification. So I simply renamed some of the conflicting functions to avoid modifying their contents (which is throw-away work)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
k,then give some comment on them to show they are throw-away work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or not, since this PR is urgent...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR types
Others
PR changes
Others
Describe
Previous works on constructing new tensor library "pten" introduced another Tensor type namely "DenseTensor", which temporarily co-exists with framework::Tensor and LoDTensor. In this series of PRs, we attempt to replace legacy "framework::Tensor and LoDTensor" by the newly implemented "DenseTensor", so as to facilitate the progress of "pten" joining original framework.
Merged C++ level LoDTensor into Tensor, so that all previous accesses to LoDTensor will be re-routed to Tensor. Same merge also happens at pybind level, where we merged all pybind methods of core.LoDTensor to core.Tensor. After that, we further patched core.LoDTensor method to core.Tensor so as to guarantee API compatibility.