Skip to content

deepSwap #565

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

Merged
merged 3 commits into from
Nov 29, 2016
Merged

deepSwap #565

merged 3 commits into from
Nov 29, 2016

Conversation

tianbingsz
Copy link
Contributor

@tianbingsz tianbingsz commented Nov 22, 2016

Fixes #566

With this PR, we can write code to directly deep swap the content of Vector and Matrix under the constraint of not allowing changing the data_ pointers.
Here is the example code to deep swap two Vectors,

para->getBuf(PARAMETER_SNAPSHOT_VALUE)->deepSwap(*para->getBuf(PARAMETER_VALUE))

and this is for the Matrices,

MatrixPtr cpuA = std::make_shared<CpuMatrix>(height, width);
MatrixPtr cpuB = std::make_shared<CpuMatrix>(height, width);
cpuA->deepSwap(*cpuB);

* swap(this, b)
* @endcode
*/
void deepSwap(BaseMatrixT& b);
Copy link
Collaborator

Choose a reason for hiding this comment

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

As here adds a new method, if we really think we need it, we'd write unit tests for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, passed deepSwap unit test test_matrixCompare

Copy link
Collaborator

@wangkuiyi wangkuiyi Nov 29, 2016

Choose a reason for hiding this comment

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

我们需要在comments里有更细致的描述,如何使用这个新增的method。我还没仔细研究如何自动从comments产生C++ API document的,可能可以问问 @hedaoyuan

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, will check with @hedaoyuan

@@ -457,6 +457,13 @@ class BaseMatrixT {

/**
* @code
* swap(this, b)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to describe how are we going to use deepswap in this usage comment, and make sure that the example code comes into our auto-generated API documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@hedaoyuan
Copy link
Contributor

hedaoyuan commented Nov 29, 2016

I think it would be better to add a swap function that can support the exchange of data_ pointer. This should be more efficient than exchange all elements in the data_.

Copy link
Collaborator

@wangkuiyi wangkuiyi left a comment

Choose a reason for hiding this comment

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

我没有问题了。但是 @hedaoyuan 有一个问题 。请 @tianbingsz 关注。

@tianbingsz
Copy link
Contributor Author

@hedaoyuan, the use case is to swap the content of vector or matrix without changing the pointers.

@tianbingsz
Copy link
Contributor Author

Just Sync with @hedaoyuan to clear the usage of this issue. Thanks for @hedaoyuan and @wangkuiyi insightful comments.

@tianbingsz tianbingsz merged commit 9e65cee into PaddlePaddle:develop Nov 29, 2016
wangxicoding pushed a commit to wangxicoding/Paddle that referenced this pull request Dec 9, 2021
* Fix dataset doc

* fix dataset doc

* Change load_dataset design for loading local dataset

* Update softmax_with_cross_entropy to cross_entropy
lizexu123 pushed a commit to lizexu123/Paddle that referenced this pull request Feb 23, 2024
tianyuzhou668 pushed a commit to tianyuzhou668/Paddle that referenced this pull request May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants