i.smap: fix possible pole error with log in extract function

[ymdatta]

Using logarithm function call with zero argument will lead to a pole error, which occurs if the mathematical function has an exact infinite result.

Refactor the conditional to only execute the code when number of subclasses are more than 1, which would preemptively stop us from getting into situation where log would have a zero argument. In cases where number of subclasses are <= 0, set the default value to 0. This is necessary as ll points to malloc'd memory and can contain renadom value.

References:

[1] https://en.cppreference.com/w/c/numeric/math/log
[2] SEI CERT C Coding Standard

[nilason]

I gather this issue originates from cppcheck, this is the relevant part for this:

cppcheck imagery/i.smap/model.c
...
imagery/i.smap/model.c:158:43: warning: Invalid log() argument nr 1. The value is 0 but the valid values are '4.94066e-324:'. [invalidFunctionArg]
                        ll[i][j][m] = log(subsum) + maxlike;
                                          ^
imagery/i.smap/model.c:154:34: note: Assignment 'subsum=0', assigned value is 0
                        subsum = 0;
                                 ^
imagery/i.smap/model.c:155:39: note: Assuming condition is false
                        for (k = 0; k < C->nsubclasses; k++)
                                      ^
imagery/i.smap/model.c:158:43: note: Invalid argument
                        ll[i][j][m] = log(subsum) + maxlike;

The static analysis, takes the path where C->nsubclasses are 0, which leaves subsum with the initialised value of 0.0. log(0) return "-infinity and raise the "divide-by-zero" floating-point exception"¹, which is the real issue here.

Now, in this branch of the if-else statement:

grass/imagery/i.smap/model.c

Lines 144 to 160 in bc9cb2a

	else {
	/* find the most likely subclass */
	for (k = 0; k < C->nsubclasses; k++) {
	if (k == 0)
	maxlike = subll[k];
	if (subll[k] > maxlike)
	maxlike = subll[k];
	}
	/* Sum weighted subclass likelihoods */
	subsum = 0;
	for (k = 0; k < C->nsubclasses; k++)
	subsum += exp(subll[k] - maxlike) * C->SubSig[k].pi;
	ll[i][j][m] = log(subsum) + maxlike;
	}
	}

there are two loops over C->nsubclasses which only does anything if C->nsubclasses > 1.

Wouldn't it better to change the else to a else if (C->nsubclasses > 1) on line 144, which guarantees that the for-loops are entered and the initialised value of subsum is altered?

Footnotes

1. man 3 log ↩

[ymdatta]

@nilason : I agree with your suggestion, it's a much cleaner way. Thanks!

I have modified the code to reflect that and I also added an else condition, which should cover the cases where nsubclasses <= 0 to set the value for ll to 0.0.