-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
supplet several interface of static Variable to consistent with dygraph Tensor #33330
supplet several interface of static Variable to consistent with dygraph Tensor #33330
Conversation
Thanks for your contribution! |
a3a6cf6
to
e684e65
Compare
… variable-keeping-with-ensor
… variable-keeping-with-ensor
… variable-keeping-with-ensor
…o/Paddle into variable-keeping-with-ensor
… variable-keeping-with-ensor
… variable-keeping-with-ensor
…_op_patch to fix ci converage
… variable-keeping-with-ensor
… variable-keeping-with-ensor
… variable-keeping-with-ensor
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
… variable-keeping-with-ensor
… variable-keeping-with-ensor
…o/Paddle into variable-keeping-with-ensor
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.
I gave some comments, but you can change in next PR since you already passed some CIs.
I will approve this PR.
@@ -0,0 +1,25 @@ | |||
/* Copyright (c) 2019 PaddlePaddle Authors. All Rights Reserved. |
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.
You can change in next PR:
2019 -> 2021
python/paddle/fluid/framework.py
Outdated
Returns a new Variable, detached from the current graph. | ||
It will share data with origin Variable and always doesn't have a Tensor copy. |
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.
"and always doesn't have a Tensor copy" -> "without tensor copy"
|
||
@prog_scope() | ||
def test_matmul(self): | ||
a = fluid.layers.data(name='a', shape=[2, 3], dtype='float32') |
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.
You can change in next PR:
use 2.0 api instead of fluid API for the new code.
@@ -345,6 +344,52 @@ def _test(): | |||
|
|||
self.assertRaises(Exception, _test) | |||
|
|||
def test_size(self): | |||
prog = paddle.static.Program() | |||
with fluid.program_guard(prog): |
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.
You can change in next PR:
use 2.0 api instead of fluid API for the new code.
x = fluid.data(name='x', shape=[3, 2, 1], dtype='float32') | ||
x.persistable = True | ||
feed_data = np.ones(shape=[3, 2, 1], dtype=np.float32) | ||
detach_x = x.detach() |
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.
Could we also test:
- Change to
detach_x
would also changex
- Change to
x
would also changedetach_x
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.
I have updated code with these tips, thank you.
… variable-keeping-with-ensor
… variable-keeping-with-ensor
… variable-keeping-with-ensor
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
python/paddle/fluid/framework.py
Outdated
|
||
Examples: | ||
.. code-block:: python | ||
:name: code-example1 |
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.
有文档预览的结果吗?麻烦将结果截图放到PR里
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.
done
class ShareDataOpMaker : public framework::OpProtoAndCheckerMaker { | ||
public: | ||
void Make() override { | ||
AddInput("Input", "The input tensor."); |
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.
The input tensor of ShareData operator?
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.
yes, used on static.Variable.detach
… variable-keeping-with-ensor
… variable-keeping-with-ensor
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
…ph Tensor (PaddlePaddle#33330) As the title
PR types
New features
PR changes
APIs
Describe
supplet several interface of static Variable to consistent with dygraph Tensor,including 1 property(ndim) and 4 methods( ndimension,dim,size,matmul,detach), the preview of api document listed below: