[libcamera-devel] [PATCH v2 5/5] ipa: ipu3: af: Simplify accumulations of y_items

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Wed, Mar 23, 2022 at 01:56:14PM +0000, Kieran Bingham via libcamera-devel wrote:
> Simplify the accumulation of the total and variance with a ternary
> operator.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> 
> This is optional really, it's only really a stylistic preference.
> 
> 
>  src/ipa/ipu3/algorithms/af.cpp | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp
> index ff5e9fb5b3c5..940ed68ea14a 100644
> --- a/src/ipa/ipu3/algorithms/af.cpp
> +++ b/src/ipa/ipu3/algorithms/af.cpp
> @@ -342,20 +342,14 @@ double Af::afEstimateVariance(Span<const y_table_item_t> y_items, bool isY1)
>  	double mean;
>  	double var_sum = 0;
>  
> -	for (auto y : y_items) {
> -		if (isY1)
> -			total += y.y1_avg;
> -		else
> -			total += y.y2_avg;
> -	}
> +	for (auto y : y_items)
> +		total += isY1 ? y.y1_avg : y.y2_avg;
>  
>  	mean = total / y_items.size();
>  
>  	for (auto y : y_items) {
> -		if (isY1)
> -			var_sum += pow((y.y1_avg - mean), 2);
> -		else
> -			var_sum += pow((y.y2_avg - mean), 2);
> +		double avg = isY1 ? y.y1_avg : y.y2_avg;
> +		var_sum += pow((avg - mean), 2);

You can drop the extra parentheses.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

By, this could also be optimized as

	double sum = 0;
	double sqr_sum = 0;

	for (auto y : y_items) {
		double avg = isY1 ? y.y1_avg : y.y2_avg;

		sum += avg;
		sqr_sum += avg * avg;
	}

	double mean = total / y_items.size();
	return sqr_sum / y_items.size() - mean * mean;

which should be more efficient, especially with a larger number of
items. The algorithm is subject to numerical instability though,
especially when the variance is small. The two-pass approach is
numerically stable (when the number of items is small).

>  	}
>  
>  	return var_sum / static_cast<double>(y_items.size());

-- 
Regards,

Laurent Pinchart