-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
deepSwap #565
Conversation
* swap(this, b) | ||
* @endcode | ||
*/ | ||
void deepSwap(BaseMatrixT& b); |
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.
As here adds a new method, if we really think we need it, we'd write unit tests for it.
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, passed deepSwap unit test test_matrixCompare
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.
我们需要在comments里有更细致的描述,如何使用这个新增的method。我还没仔细研究如何自动从comments产生C++ API document的,可能可以问问 @hedaoyuan 。
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.
OK, will check with @hedaoyuan
@@ -457,6 +457,13 @@ class BaseMatrixT { | |||
|
|||
/** | |||
* @code | |||
* swap(this, b) |
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.
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.
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
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_. |
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.
我没有问题了。但是 @hedaoyuan 有一个问题 。请 @tianbingsz 关注。
@hedaoyuan, the use case is to swap the content of vector or matrix without changing the pointers. |
Just Sync with @hedaoyuan to clear the usage of this issue. Thanks for @hedaoyuan and @wangkuiyi insightful comments. |
* Fix dataset doc * fix dataset doc * Change load_dataset design for loading local dataset * Update softmax_with_cross_entropy to cross_entropy
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,
and this is for the Matrices,