Removed copy constructor and assignment operator from top-level Matri…

…x type to prevent unwanted silent deep copies of Matrix objects which can be very expensive. This revealed several instances of unwanted copying of entire matrices that have also been fixed as part of this change

Source/Common/Include/Sequences.h

@@ -109,17 +109,54 @@ struct MBLayout
 
     // copy the content of another MBLayoutPtr over
     // Use this instead of actual assignment to make it super-obvious that this is not copying the pointer but actual content. The pointer is kept fixed.
-    void CopyFrom(const MBLayoutPtr &other)
+    void CopyFrom(const MBLayoutPtr& other)
     {
-        *this = *other;
+        m_numTimeSteps = other->m_numTimeSteps;
+        m_numParallelSequences = other->m_numParallelSequences;
+        m_sequences = other->m_sequences;
+        m_numFramesDeclared = other->m_numFramesDeclared;
+        m_numGapFrames = other->m_numFramesDeclared;
+
+        m_distanceToStart.SetValue(other->m_distanceToStart);
+        m_distanceToEnd.SetValue(other->m_distanceToEnd);
+
+        m_distanceToNearestStart = other->m_distanceToNearestStart;
+        m_distanceToNearestEnd = other->m_distanceToNearestEnd;
+
+        m_timeStepHasGap = other->m_timeStepHasGap;
+
+        m_columnsValidityMask.SetValue(other->m_columnsValidityMask);
+        m_writable = other->m_writable;
     }
+
+    // Destructive copy that steals ownership if the content, like std::move()
+    // Note: For some reason the VC++ compiler does not generate the 
+    // move assignment and we have to do this ourselves
     void MoveFrom(MBLayoutPtr other)
     {
-        *this = move(*other);
+        m_numTimeSteps = other->m_numTimeSteps;
+        m_numParallelSequences = other->m_numParallelSequences;
+        m_sequences = std::move(other->m_sequences);
+        m_numFramesDeclared = other->m_numFramesDeclared;
+        m_numGapFrames = other->m_numFramesDeclared;
+
+        m_distanceToStart = std::move(other->m_distanceToStart);
+        m_distanceToEnd = std::move(other->m_distanceToEnd);
+
+        m_distanceToNearestStart = std::move(other->m_distanceToNearestStart);
+        m_distanceToNearestEnd = std::move(other->m_distanceToNearestEnd);
+
+        m_timeStepHasGap = std::move(other->m_timeStepHasGap);
+
+        m_columnsValidityMask = std::move(other->m_columnsValidityMask);
+        m_writable = other->m_writable;
+
         other->Init(0, 0);
-    } // destructive copy that steals ownership if the content, like std::move()
-private:
-    MBLayout &operator=(const MBLayout &) = default; // make this private --use CopyFrom() instead, which makes it very clear that it's copying content, not copying the reference
+    }
+
+    MBLayout(const MBLayout&) = delete;
+    MBLayout& operator=(const MBLayout&) = delete;
+
 public:
     // resize and reset all frames to None (note: this is an invalid state and must be fixed by caller afterwards)
     void Init(size_t numParallelSequences, size_t numTimeSteps)

Source/ComputationNetworkLib/ComputationNetwork.cpp

@@ -913,7 +913,7 @@ void ComputationNetwork::PerformSVDecomposition(const map<wstring, float>& SVDCo
             shared_ptr<ComputationNode<ElemType>> pNode = dynamic_pointer_cast<LearnableParameter<ElemType>>(m_nameToNodeMap[name]);
 
             // Step 1. do SVD decomposition
-            Matrix<ElemType> A = pNode->ValueAsMatrix();
+            Matrix<ElemType> A = pNode->ValueAsMatrix().DeepClone();
 
             // it is a vector, no need to do it
             if (A.GetNumCols() == 1 || A.GetNumRows() == 1)
@@ -991,8 +991,8 @@ void ComputationNetwork::PerformSVDecomposition(const map<wstring, float>& SVDCo
             shared_ptr<ComputationNode<ElemType>> pLeft = AddNodeToNetWithElemType(New<LearnableParameter<ElemType>>(m_deviceId, leftChildName, m, r));
             shared_ptr<ComputationNode<ElemType>> pRight = AddNodeToNetWithElemType(New<LearnableParameter<ElemType>>(m_deviceId, rightChildName, r, n));
 
-            pLeft->ValueAsMatrix() = redU;
-            pRight->ValueAsMatrix() = redVT;
+            pLeft->ValueAsMatrix() = std::move(redU);
+            pRight->ValueAsMatrix() = std::move(redVT);
 
             shared_ptr<ComputationNode<ElemType>> pTimes = AddNodeToNetAndAttachInputs(New<TimesNode<ElemType>>(m_deviceId, name + L"-SVD"), pLeft, pRight);
 
@@ -1212,9 +1212,8 @@ void ComputationNetwork::SaveToDbnFile(ComputationNetworkPtr net, const std::wst
     ComputationNodeBasePtr meanNode = normalizationNodes.front()->GetInputs()[1];
     ComputationNodeBasePtr stdNode = normalizationNodes.front()->GetInputs()[2];
 
-    Matrix<ElemType> meanNodeMatrix = meanNode->As<ComputationNode<ElemType>>()->Value();
-    Matrix<ElemType> stdNodeMatrix = stdNode->As<ComputationNode<ElemType>>()->Value();
-    Matrix<ElemType> invStdNodeMatrix(stdNodeMatrix.ElementInverse());
+    Matrix<ElemType> meanNodeMatrix = meanNode->As<ComputationNode<ElemType>>()->Value().DeepClone();
+    Matrix<ElemType> invStdNodeMatrix(std::move(stdNode->As<ComputationNode<ElemType>>()->Value().DeepClone().ElementInverse()));
 
     std::vector<ComputationNodeBasePtr> priorNodes = WhereNode(net->GetAllNodes(), GetAllPriorNodes);
     if (priorNodes.size() != 1)
@@ -1297,7 +1296,7 @@ void ComputationNetwork::SaveToDbnFile(ComputationNetworkPtr net, const std::wst
         }
 
         // Write out the main weight matrix
-        auto weight = (layer->Node->As<ComputationNode<ElemType>>()->Value());
+        auto weight = (layer->Node->As<ComputationNode<ElemType>>()->Value().DeepClone());
         auto transpose = weight.Transpose();
         PutMatrix(&transpose, "W");
 

Source/ComputationNetworkLib/ComputationNode.h

@@ -907,9 +907,9 @@ class ComputationNode : public ComputationNodeBase // abstract class that cannot
         if (flags & CopyNodeFlags::copyNodeValue)
         {
             auto node = DownCast(nodeP);
-            *node->m_value = *m_value;
+            node->m_value->SetValue(*m_value);
             if (m_gradient)
-                *node->m_gradient = *m_gradient;
+                node->m_gradient->SetValue(*m_gradient);
             else
                 node->m_gradient = nullptr;
         }

Source/ComputationNetworkLib/ConvolutionalNodes.h

@@ -144,7 +144,7 @@ class ConvolutionNode : public ComputationNode<ElemType>, public NumInputs<2>
 
             node->m_imageLayoutKind = m_imageLayoutKind;
 
-            *node->m_tempMatrix = *m_tempMatrix;
+            node->m_tempMatrix->SetValue(*m_tempMatrix);
         }
     }
 

Source/ComputationNetworkLib/EvaluationNodes.h

@@ -101,9 +101,9 @@ class ErrorPredictionNode : public ComputationNodeNonLooping /*ComputationNode*/
         if (flags & CopyNodeFlags::copyNodeValue)
         {
             auto node = dynamic_pointer_cast<ErrorPredictionNode<ElemType>>(nodeP);
-            *node->m_maxIndexes0 = *m_maxIndexes0;
-            *node->m_maxIndexes1 = *m_maxIndexes1;
-            *node->m_maxValues = *m_maxValues;
+            node->m_maxIndexes0->SetValue(*m_maxIndexes0);
+            node->m_maxIndexes1->SetValue(*m_maxIndexes1);
+            node->m_maxValues->SetValue(*m_maxValues);
         }
     }
     // request matrices needed to do node function value evaluation

Source/ComputationNetworkLib/LinearAlgebraNodes.h

@@ -556,8 +556,8 @@ class DiagTimesNode : public ComputationNode<ElemType>, public NumInputs<2>
         if (flags & CopyNodeFlags::copyNodeValue)
         {
             auto node = dynamic_pointer_cast<DiagTimesNode<ElemType>>(nodeP);
-            *node->m_innerproduct = *m_innerproduct;
-            *node->m_rightGradient = *m_rightGradient;
+            node->m_innerproduct->SetValue(*m_innerproduct);
+            node->m_rightGradient->SetValue(*m_rightGradient);
         }
     }
     // request matrices that are needed for gradient computation
@@ -895,11 +895,11 @@ class CosDistanceNode : public ComputationNode<ElemType>, public NumInputs<2>
         if (flags & CopyNodeFlags::copyNodeValue)
         {
             auto node = dynamic_pointer_cast<CosDistanceNode<ElemType>>(nodeP);
-            *node->m_invNorm0 = *m_invNorm0;
-            *node->m_invNorm1 = *m_invNorm1;
-            *node->m_leftTerm = *m_leftTerm;
-            *node->m_rightTerm = *m_rightTerm;
-            *node->m_temp = *m_temp;
+            node->m_invNorm0->SetValue(*m_invNorm0);
+            node->m_invNorm1->SetValue(*m_invNorm1);
+            node->m_leftTerm->SetValue(*m_leftTerm);
+            node->m_rightTerm->SetValue(*m_rightTerm);
+            node->m_temp->SetValue(*m_temp);
         }
     }
     // request matrices needed to do node function value evaluation
@@ -1212,12 +1212,12 @@ class CosDistanceWithNegativeSamplesNode : public ComputationNode<ElemType>, pub
         if (flags & CopyNodeFlags::copyNodeValue)
         {
             auto node = dynamic_pointer_cast<CosDistanceWithNegativeSamplesNode<ElemType>>(nodeP);
-            *node->m_invNorm0 = *m_invNorm0;
-            *node->m_invNorm1 = *m_invNorm1;
-            *node->m_invNormSquare = *m_invNormSquare;
-            *node->m_leftTerm = *m_leftTerm;
-            *node->m_rightTerm = *m_rightTerm;
-            *node->m_temp = *m_temp;
+            node->m_invNorm0->SetValue(*m_invNorm0);
+            node->m_invNorm1->SetValue(*m_invNorm1);
+            node->m_invNormSquare->SetValue(*m_invNormSquare);
+            node->m_leftTerm->SetValue(*m_leftTerm);
+            node->m_rightTerm->SetValue(*m_rightTerm);
+            node->m_temp->SetValue(*m_temp);
         }
     }
     // request matrices needed to do node function value evaluation

Source/ComputationNetworkLib/NonlinearityNodes.h

@@ -180,7 +180,7 @@ class SoftmaxNodeBase : public ComputationNode<ElemType>, public NumInputs<1>
         if (flags & CopyNodeFlags::copyNodeValue)
         {
             auto node = dynamic_pointer_cast<SoftmaxNodeBase<ElemType>>(nodeP);
-            *node->m_gradientTemp = *m_gradientTemp;
+            node->m_gradientTemp->SetValue(*m_gradientTemp);
         }
     }
 
@@ -258,7 +258,7 @@ class SoftmaxNode : public SoftmaxNodeBase<ElemType>
         if (flags & CopyNodeFlags::copyNodeValue)
         {
             auto node = dynamic_pointer_cast<SoftmaxNode<ElemType>>(nodeP);
-            *node->m_diff = *m_diff;
+            node->m_diff->SetValue(*m_diff);
         }
     }
     // request matrices that are needed for gradient computation
@@ -331,7 +331,7 @@ class LogSoftmaxNode : public SoftmaxNodeBase<ElemType>
         if (flags & CopyNodeFlags::copyNodeValue)
         {
             auto node = dynamic_pointer_cast<LogSoftmaxNode<ElemType>>(nodeP);
-            *node->m_softmax = *m_softmax;
+            node->m_softmax->SetValue(*m_softmax);
         }
     }
     // request matrices that are needed for gradient computation

Source/ComputationNetworkLib/PreComputeNodes.h

@@ -398,9 +398,9 @@ class InvStdDevNode : public MeanInvStdDevNodeBase<ElemType>
         if (flags & CopyNodeFlags::copyNodeValue)
         {
             auto node = dynamic_pointer_cast<InvStdDevNode<ElemType>>(nodeP);
-            node->m_mean = m_mean;
-            node->m_var = m_var;
-            node->m_temp = m_temp;
+            node->m_mean.SetValue(m_mean);
+            node->m_var.SetValue(m_var);
+            node->m_temp.SetValue(m_temp);
         }
     }
 

Source/ComputationNetworkLib/RecurrentNodes.h

@@ -254,7 +254,7 @@ class DelayedValueNodeBase : public ComputationNode<ElemType>, public IRecurrent
         //  - we don't need to keep anything if all sequences are closed (sentence end)
         //    This condition includes full-sequence mode.
         // TODO: Can we optimize this and only copy if there is a sequence spanning across the end of the MB? And add a check to BeginForwardProp() to make sure we got one if there is a boundary at the start?
-        m_delayedValue = Input(0)->Value();
+        m_delayedValue.SetValue(Input(0)->Value());
         if (!m_delayedActivationMBLayout)
             m_delayedActivationMBLayout = make_shared<MBLayout>();
         m_delayedActivationMBLayout->CopyFrom(m_pMBLayout);
@@ -379,7 +379,7 @@ class DelayedValueNodeBase : public ComputationNode<ElemType>, public IRecurrent
             auto node = dynamic_pointer_cast<DelayedValueNodeBase<ElemType, direction /*, SequenceStart_or_End*/>>(nodeP);
             node->m_timeStep = m_timeStep;
             node->m_initialActivationValue = m_initialActivationValue;
-            node->m_delayedValue = m_delayedValue;
+            node->m_delayedValue.SetValue(m_delayedValue);
             if (m_delayedActivationMBLayout)
                 (node->m_delayedActivationMBLayout = make_shared<MBLayout>())->CopyFrom(m_delayedActivationMBLayout);
             else

Source/ComputationNetworkLib/ReshapingNodes.h

@@ -621,7 +621,7 @@ class DiagonalNode : public ComputationNodeNonLooping<ElemType>, public NumInput
         // BUGBUG: This should use the memshare mechanism.
         // TODO: use tensor lib, then this will be easy, no memsharing needed
         Matrix<ElemType> diag(gradientValues.GetNumRows(), gradientValues.GetNumCols(), gradientValues.GetDeviceId());
-        diag = gradientValues;
+        diag.SetValue(gradientValues);
         diag.Resize(gradientValues.GetNumCols(), 1);
 
         inputGradientValues.SetValue(0);

Source/ComputationNetworkLib/SpecialPurposeNodes.h

@@ -564,9 +564,9 @@ class SequenceWithSoftmaxNode : public ComputationNodeNonLooping<ElemType>, publ
         {
             auto node = dynamic_pointer_cast<SequenceWithSoftmaxNode<ElemType>>(nodeP);
 
-            *node->m_logSoftmaxOfRight = *m_logSoftmaxOfRight;
-            *node->m_softmaxOfRight = *m_softmaxOfRight;
-            *node->m_gammaFromLattice = *m_gammaFromLattice;
+            node->m_logSoftmaxOfRight->SetValue(*m_logSoftmaxOfRight);
+            node->m_softmaxOfRight->SetValue(*m_softmaxOfRight);
+            node->m_gammaFromLattice->SetValue(*m_gammaFromLattice);
             node->m_fsSmoothingWeight = m_fsSmoothingWeight;
             node->m_frameDropThreshold = m_frameDropThreshold;
             node->m_doReferenceAlignment = m_doReferenceAlignment;

Source/ComputationNetworkLib/TrainingNodes.h

@@ -85,7 +85,7 @@ class SquareErrorNode : public ComputationNodeNonLooping /*ComputationNode*/<Ele
         if (flags & CopyNodeFlags::copyNodeValue)
         {
             auto node = dynamic_pointer_cast<SquareErrorNode<ElemType>>(nodeP);
-            *node->m_leftMinusRight = *m_leftMinusRight;
+            node->m_leftMinusRight->SetValue(*m_leftMinusRight);
         }
     }
 
@@ -214,8 +214,8 @@ class CrossEntropyWithSoftmaxNode : public ComputationNodeNonLooping /*Computati
         if (flags & CopyNodeFlags::copyNodeValue)
         {
             auto node = dynamic_pointer_cast<CrossEntropyWithSoftmaxNode<ElemType>>(nodeP);
-            *node->m_logSoftmaxOfRight = *m_logSoftmaxOfRight;
-            *node->m_softmaxOfRight = *m_softmaxOfRight;
+            node->m_logSoftmaxOfRight->SetValue(*m_logSoftmaxOfRight);
+            node->m_softmaxOfRight->SetValue(*m_softmaxOfRight);
         }
     }
 
@@ -325,8 +325,8 @@ class CrossEntropyNode : public ComputationNodeNonLooping /*ComputationNode*/<El
         if (flags & CopyNodeFlags::copyNodeValue)
         {
             auto node = dynamic_pointer_cast<CrossEntropyNode<ElemType>>(nodeP);
-            *node->m_logOfRight = *m_logOfRight;
-            *node->m_leftDivRight = *m_leftDivRight;
+            node->m_logOfRight->SetValue(*m_logOfRight);
+            node->m_leftDivRight->SetValue(*m_leftDivRight);
         }
     }
 
@@ -430,7 +430,7 @@ class MatrixL1RegNode : public ComputationNodeNonLooping /*ComputationNode*/<Ele
         if (flags & CopyNodeFlags::copyNodeValue)
         {
             auto node = dynamic_pointer_cast<MatrixL1RegNode<ElemType>>(nodeP);
-            *node->m_gradientOfL1Norm = *m_gradientOfL1Norm;
+            node->m_gradientOfL1Norm->SetValue(*m_gradientOfL1Norm);
         }
     }
 
@@ -860,7 +860,7 @@ class ClassBasedCrossEntropyWithSoftmaxNode : public ComputationNodeNonLooping /
         assert(m_nbrCls == Input(CLASSPROBINDATA)->GetSampleMatrixNumRows());
 
         // compute the class posteriors
-        m_clsLogSoftmax = Input(CLASSPROBINDATA)->Value();
+        m_clsLogSoftmax.SetValue(Input(CLASSPROBINDATA)->Value());
         m_clsLogSoftmax.InplaceLogSoftmax(true);   // log
         m_clsSoftmax.AssignExpOf(m_clsLogSoftmax); // non-log
 
@@ -1330,7 +1330,7 @@ class LogisticNode : public ComputationNodeNonLooping /*ComputationNode*/<ElemTy
         const Matrix<ElemType>& classOneProbabilities = Input(1)->ValueFor(fr);
         Matrix<ElemType>& classZeroLabels = *m_classZeroLabels;
 
-        Matrix<ElemType> ones = ConstOnes(classOneLabels.GetNumRows(), classOneLabels.GetNumCols(), classOneLabels.GetDeviceId());
+        Matrix<ElemType> ones = ConstOnes(classOneLabels.GetNumRows(), classOneLabels.GetNumCols(), classOneLabels.GetDeviceId()).DeepClone();
 
         // compute the indices for the class 0 indices
         classZeroLabels.AssignDifferenceOf(ones, classOneLabels);
@@ -1409,9 +1409,9 @@ class LogisticNode : public ComputationNodeNonLooping /*ComputationNode*/<ElemTy
         if (flags & CopyNodeFlags::copyNodeValue)
         {
             auto node = dynamic_pointer_cast<LogisticNode<ElemType>>(nodeP);
-            *node->m_classZeroLabels = *m_classZeroLabels;
-            *node->m_result = *m_result;
-            *node->m_temp = *m_temp;
+            node->m_classZeroLabels->SetValue(*m_classZeroLabels);
+            node->m_result->SetValue(*m_result);
+            node->m_temp->SetValue(*m_temp);
         }
     }