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

[Kieran Bingham]

Quoting Laurent Pinchart (2022-03-24 01:06:20)
> 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).

I did wonder if we could get this in a single loop, but I didn't want to
break any assumptions that were made on precision or overflows etc, by
making fundamental changes to the operation of the code.

I presume that when the variance is small, as a double has 15 decimal
points I expect we're way into insignificant numbers that don't affect
the overall calculation.

Kate, JM, what do you think? Any preference on any of the above?

--
Kieran


> 
> >       }
> >  
> >       return var_sum / static_cast<double>(y_items.size());
> 
> -- 
> Regards,
> 
> Laurent Pinchart