NCC::predict is incorrect for weights (not equal zero or unity)

[Peter]

related to ticket:210

void NCC::predict(const DataLookup2D& input, utility::matrix& prediction) const

The problem occurs in this snippet

for(size_t j=0; j<input.columns();j++) { 		 
				DataLookup1D in(input,j,false);
				utility::vector weights(in.size(),0);
				for(size_t i=0; i<in.size();i++) 
					weights(i)=data->weight(i,j);
				utility::vector out;
				predict(in,weights,out);
				prediction.column(j,out);
			}

The DataLookup1D in is lookup into a MatrixLookupWeighted?. When calling in(i), MatrixLookupWeighted::operator() is called and returns x*w for the element in question. On the other, weight is also provided to predict(const DataLookup1D& input, const utility::vector& weights. This results in that instead of having x and w vectors we have vectors x*w and w.

[Peter]

The quickest fix would be to rather than creating the DataLookup1D as DataLookup1D in(input,j,false); to first create a utility::vector, fill it with data->value(i,j), and then create a DataLookup1D from the utility::vector.

The bad thing about this solution is COPYING. In fact, we would copy each value in the vector twice. First when filling the utility::vector, second when calling constructor DataLookup1D(const utility::vector&) (DataLookup1D holds a pointer to a DataLookup2D that holds a pointer to a utility::matrix. This matrix is copied from the utility::vector.).

Next suggestion... well I forgot that one, but another solution would of course be to do a larger redesign. I don't know but perhaps we should change the Distance interface. Probably not since it is released... but I wanted to bring up the idea.

[Markus Ringnér]

There function value above is actually called data so the quick fix described should be using data->data(i,j) instead.

[Peter]

Sure.

Also have a look at ticket:210