Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

fix code problem #1858

Merged
merged 1 commit into from
Apr 19, 2016
Merged

fix code problem #1858

merged 1 commit into from
Apr 19, 2016

Conversation

yajiedesign
Copy link
Contributor

No description provided.

@yajiedesign yajiedesign force-pushed the cleancode branch 2 times, most recently from b29ecb2 to 92c2300 Compare April 15, 2016 23:52
@yajiedesign
Copy link
Contributor Author

yajiedesign commented Apr 16, 2016

@Javelinjs some test error with Scala
src/operator/operator_util.cc:611: Check failed: (lhs.ctx()) == (rhs.ctx()) operands context mismatch (4,5) vs. (4,5)
Is not because the model parallel, it has no need to detect whether the context is the same?

@@ -106,7 +106,7 @@ struct ImageAugmentParam : public dmlc::Parameter<ImageAugmentParam> {
class ImageAugmenter {
public:
// contructor
ImageAugmenter(void) {
ImageAugmenter(void):param_() {
Copy link
Member

Choose a reason for hiding this comment

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

implicit constructor do not have to be called

@yajiedesign yajiedesign force-pushed the cleancode branch 2 times, most recently from a78399b to 5c0a8b6 Compare April 16, 2016 06:38
@@ -608,9 +608,9 @@ void SimpleOpRegEntryImpl::RegisterBinaryImperative() {
CHECK_EQ(lhs.shape(), rhs.shape()) << "operands shape mismatch";
dshape = lhs.shape();
}
CHECK_EQ(lhs.ctx(), lhs.ctx())
CHECK_EQ(lhs.ctx(), rhs.ctx())
<< "operands context mismatch " << lhs.shape() << " vs. " << rhs.shape();
Copy link
Member

Choose a reason for hiding this comment

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

change shape to context

@tqchen
Copy link
Member

tqchen commented Apr 17, 2016

The scalar error seems to due to problem of using binary op for tensors from different devices, which is forbided and detected by the changes @yajiedesign added.

@Javelinjs can you look a bit into it? @yajiedesign it is also OK if you can simply remove that testcase for now

@yzhliu
Copy link
Member

yzhliu commented Apr 18, 2016

Sorry for the late reply. @yajiedesign Could you help to patch the following diff?

diff --git a/scala-package/core/src/test/scala/ml/dmlc/mxnet/ModelParallelSuite.scala b/scala-package/core/src/test/scala/ml/dmlc/mxnet/ModelParallelSuite.scala
index afbcdca..48dfaef 100644
--- a/scala-package/core/src/test/scala/ml/dmlc/mxnet/ModelParallelSuite.scala
+++ b/scala-package/core/src/test/scala/ml/dmlc/mxnet/ModelParallelSuite.scala
@@ -47,7 +47,8 @@ class ModelParallelSuite extends FunSuite with BeforeAndAfterAll {

     exec1.forward()
     exec2.forward()
-    assert(reldiff(exec1.outputs(0), exec2.outputs(0)) < 1e-6f)
+    assert(reldiff(exec1.outputs(0).copyTo(Context.cpu()),
+      exec2.outputs(0).copyTo(Context.cpu())) < 1e-6f)

     val outGrad = NDArray.ones(shape, Context.cpu(1))
     exec1.backward(Array(outGrad))

@yajiedesign
Copy link
Contributor Author

@Javelinjs yes ok

@tqchen tqchen merged commit 208ce14 into apache:master Apr 19, 2016
@yajiedesign yajiedesign deleted the cleancode branch May 25, 2016 13:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants