Trouble with sqrt & template functions

Started by
25 comments, last by jmau0438 14 years, 7 months ago
Hello out there, I'm writting a stats utility header filled with templated functions, and I've run into some trouble when I try using the sqrt function in <math.h>. Here is the function:

template < typename TYPE > TYPE STD_DEVIATION ( const TYPE *Array, const int Length ) {

		//Declare & Initialize the mean to the average of the sample
		TYPE Mean = MEAN < TYPE > ( Array, Length );

		//Declare an array containing ( X - M ) data
		TYPE *X_M = new TYPE [ Length ];

		//Declare an array containing ( X - M ) 2 data
		TYPE *X_M_SQRT = new TYPE [ Length ];

		//Declare a variable to store the sum of ( X - m ) 2 & Initialize to zero
		TYPE X_M_SQRT_SUM = 0;

		//For each value within a sample, calculate & store the mean & deviation by...
		for ( int intLoop = 0; intLoop < Length; intLoop++ ) {

			//...subtracting the mean from the current sample value & store it in the ( X - M ) array...
			X_M [ intLoop ] = ( Array [ intLoop ] - Mean );

			//...and squaring the result of the previous calculation & store it in the ( X - M ) 2 array...
			X_M_SQRT [ intLoop ] = ( sqrt ( X_M [ intLoop ] ) );

		};

		//Calculate the sum of ( X - M ) 2
		for ( int intLoop = 0; intLoop < Length; intLoop++ ) { X_M_SQRT_SUM += X_M_SQRT [ intLoop ]; };

		//Purge the ( X - M ) & ( X - M ) 2 arrays from memory
		delete X_M, X_M_SQRT;
		X_M, X_M_SQRT = 0;

		//Return the standard deviation
		return ( sqrt ( X_M_SQRT_SUM ) / sqrt ( Length - 1 ) );

	};


HERE IS THE ERROR error C2668: 'sqrt' : ambiguous call to overloaded function I've tried hacking in some type casting tricks, but i usually end up with a -1.#IND result, which I'm pretty sure is a type of NAN value. This would make sense if I was trying to square zero or maybe even a negative number, but I know I'm not. I know much of the problem revolves around the fact that I'm using a templated funtion and the compiler doesn't know the type during compilation, which is giving me this error. Is what I'm trying just not possible, or is there a work around? [Edited by - Zahlman on August 19, 2009 9:10:36 AM]
Advertisement
The compiler knows the type, because templates aren't actually "compiled" when the compiler sees them. Rather, the compiler just holds onto the code, and it's only when the template is "instantiated" that the compiler fills in all the blanks and can generate code.

The problem is that you're supplying an integer to the sqrt function (when you perform sqrt(Length - 1)). Now, sqrt can take a float or a double. The compiler needs to convert your int into a float or a double - but which one? The compiler can't make this decision, and so it gives you an "ambiguous call" error.
NextWar: The Quest for Earth available now for Windows Phone 7.
You are 100% right, thanks! I changed the 1 to 1.0f and it worked! I'm still gettin that -1.#IND result, any thoughts?
Never mind, I just found out that I was squaring a negative number. Oops!! Anyway, thanks so much for your help Sc4Freak, I really appreciate it!!
Quote:Original post by jmau0438
Never mind, I just found out that I was squaring a negative number. Oops!! Anyway, thanks so much for your help Sc4Freak, I really appreciate it!!

Yes, on this line:

X_M_SQRT [ intLoop ] = ( sqrt ( X_M [ intLoop ] ) );

You're supposed to square the number, not square root.

There are some other things I'd like to mention as a sidenote, however:
1) Use the [source][/source] tags to format your code nicely
2) You're doing a lot of unneccessary calculation. The following code does the same thing and is a lot more clearer and compact:
template<typename T>T StdDev(const T* data, size_t num){    const T* end = data + num;    // If you don't know what std::accumulate does, it just adds stuff in    // the given range    T mean = std::accumulate(data, end, 0) / num;    T sum = 0;    for(; data != end; ++data)    {        T diff = *data - mean;        sum += diff * diff;    }    sum /= num;    return static_cast<T>(sqrt(static_cast<double>(sum)));}

No memory allocation needed, and just one loop.
3) You leak memory. You allocate the arrays X_M and X_M_SQRT, but you call delete. You need to call delete[] if you allocated using new[].
NextWar: The Quest for Earth available now for Windows Phone 7.
Quote:Original post by jmau0438
//Purge the ( X - M ) & ( X - M ) 2 arrays from memory
delete X_M, X_M_SQRT;
X_M, X_M_SQRT = 0;


Unfortunately, these lines do not do what you expect. They're equivalent to the following:

delete X_M;X_M_SQRT;    // Does nothingX_M;         // Does nothingX_M_SQRT = 0;


You have to perform each action individually, as follows (incorporating Sc4Freak's delete[] fix):

delete[] X_M, delete[] X_M_SQRT;X_M = 0, X_M_SQRT = 0;


The comma separates statements (just like the semi-colon). It does not apply one operator to multiple values.
Hi

Also, you don't in general need to put semicolons after a set of curly braces - only if it's a struct/class definition.

for(int i=0;i<maxloop;++i){    //Do stuff    //Do more stuff}


will work fine. Curly brackets are meant to group statements into a block - they aren't statements in themselves, so they don't need to be finished with the semicolon.
Quote:delete X_M, X_M_SQRT;
As said, this doesn't work.

Unless you use the improved version with no allocation, use something like this instead:
//Declare an array containing ( X - M ) datastd::vector<TYPE> X_M(length);//Declare an array containing ( X - M ) 2 datastd::vector<TYPE> X_M_SQRT(length);
Quote:Original post by Sc4Freak
3) You leak memory. You allocate the arrays X_M and X_M_SQRT, but you call delete. You need to call delete[] if you allocated using new[].


That's not a memory leak; it's undefined behaviour.

And as long as we're showing off templates and the standard library algorithms, why not adapt the interface to follow the standard library conventions as well? :)

Oh, and in case the input type is an integer, you might not want to be doing integer division all over. Fortunately, there is an algebraic simplication which gets around that...

When we calculate the squared sum, we calculate the sum of all (x - x0)2, i.e. the sum of all (x2 - 2x*x0 + x02). Those sums are separable, and the sum of 2x*x0 is 2x0 times the sum of the x values; but since x0 is the mean value, the sum of the x values is simply x0 * N.

So we end up with the (sum of x2) - 2N x02 + N x02, or just (sum of x2) - N x02. And since (sum of x) == (N x0), again, we can simplify as (sum of x2) - ((sum of x)2 / N). :)

Finally, it doesn't really make sense to force the result type back to the same as the element type. The standard deviation is intrinsically a decimal quantity.

template <typename ForwardIterator> double std_deviation(ForwardIterator begin, ForwardIterator end) {  double N = static_cast<double>(std::distance(begin, end));  typedef typename ForwardIterator::value_type T;  T sum = std::accumulate(begin, end, 0);  T sum_of_squares = 0;  for (; begin != end; ++begin) {    sum_of_squares += *begin * *begin;  }  // Now all the division comes at the end.  return sqrt(    static_cast<double>(sum_of_squares) -    static_cast<double>(sum * sum) / N  ) / sqrt(N - 1);}
Wow, you guys are awesome. I guess I respond in parts:

Sc4Freak - Square not Square Root

I was to focused on the square root problem. After you showed me how to fix the sqrt() function call, it took but a few seconds for me to realize that when I performed the square root operation (which is flat wrong at this stage in the formula) that I was creating negative numbers, which gave me the NAN value (1.#IND).

Sc4Freak, scjohnno, Zahlman, & Antheus - Memory Leak

Ya, I caught the memory leak too. Again, I was way to focused on the sqrt() problem so I didn't catch it until last night. Sloppy I know, but hey, it was 4 in the morning. I simplified the code a bit so I'm only using a single local array, and replaced the previous deletion with this:

//Purge the ( X - M ) array from memory
delete [] X_M, X_M = 0;

shaolinspin - Semi-colon after curly brace

I know about the semi-colon rule, but it was a bad habit I picked up in college. To this day I habitually throw a semi-colon behind closing curly braces. It haven't experienced any trouble caused by the habit, but just out of curiosity are there any adverse effects to adding the semi-colon?

This topic is closed to new replies.

Advertisement