| Message ID | 20231206172743.13355-2-nick.hollinghurst@raspberrypi.com |
|---|---|
| State | Accepted |
| Commit | e71d63ce1bfa9784e18852fec264357bda5e3671 |
| Headers | show |
| Series |
|
| Related | show |
Hi Nick Thanks for the patch. Looks good to me! On Wed, 6 Dec 2023 at 17:27, Nick Hollinghurst via libcamera-devel <libcamera-devel@lists.libcamera.org> wrote: > > Don't assert when taking the weighted mean of a zero-width or > zero-weight interval; return its upper bound. That is certainly > correct in the zero-width case, and plausible otherwise. > > Signed-off-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com> Reviewed-by: David Plowman <david.plowman@raspberrypi.com> Thanks! David > --- > src/ipa/rpi/controller/histogram.cpp | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/src/ipa/rpi/controller/histogram.cpp b/src/ipa/rpi/controller/histogram.cpp > index 0a27ba2c..78116141 100644 > --- a/src/ipa/rpi/controller/histogram.cpp > +++ b/src/ipa/rpi/controller/histogram.cpp > @@ -47,7 +47,7 @@ double Histogram::quantile(double q, int first, int last) const > > double Histogram::interBinMean(double binLo, double binHi) const > { > - assert(binHi > binLo); > + assert(binHi >= binLo); > double sumBinFreq = 0, cumulFreq = 0; > for (double binNext = floor(binLo) + 1.0; binNext <= ceil(binHi); > binLo = binNext, binNext += 1.0) { > @@ -57,13 +57,19 @@ double Histogram::interBinMean(double binLo, double binHi) const > sumBinFreq += bin * freq; > cumulFreq += freq; > } > + > + if (cumulFreq == 0) { > + /* interval had zero width or contained no weight? */ > + return binHi; > + } > + > /* add 0.5 to give an average for bin mid-points */ > return sumBinFreq / cumulFreq + 0.5; > } > > double Histogram::interQuantileMean(double qLo, double qHi) const > { > - assert(qHi > qLo); > + assert(qHi >= qLo); > double pLo = quantile(qLo); > double pHi = quantile(qHi, (int)pLo); > return interBinMean(pLo, pHi); > -- > 2.39.2 >
Hi Nick, Thanks for the patch. On Wed, 6 Dec 2023 at 17:27, Nick Hollinghurst via libcamera-devel <libcamera-devel@lists.libcamera.org> wrote: > > Don't assert when taking the weighted mean of a zero-width or > zero-weight interval; return its upper bound. That is certainly > correct in the zero-width case, and plausible otherwise. > > Signed-off-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com> Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/ipa/rpi/controller/histogram.cpp | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/src/ipa/rpi/controller/histogram.cpp b/src/ipa/rpi/controller/histogram.cpp > index 0a27ba2c..78116141 100644 > --- a/src/ipa/rpi/controller/histogram.cpp > +++ b/src/ipa/rpi/controller/histogram.cpp > @@ -47,7 +47,7 @@ double Histogram::quantile(double q, int first, int last) const > > double Histogram::interBinMean(double binLo, double binHi) const > { > - assert(binHi > binLo); > + assert(binHi >= binLo); > double sumBinFreq = 0, cumulFreq = 0; > for (double binNext = floor(binLo) + 1.0; binNext <= ceil(binHi); > binLo = binNext, binNext += 1.0) { > @@ -57,13 +57,19 @@ double Histogram::interBinMean(double binLo, double binHi) const > sumBinFreq += bin * freq; > cumulFreq += freq; > } > + > + if (cumulFreq == 0) { > + /* interval had zero width or contained no weight? */ > + return binHi; > + } > + > /* add 0.5 to give an average for bin mid-points */ > return sumBinFreq / cumulFreq + 0.5; > } > > double Histogram::interQuantileMean(double qLo, double qHi) const > { > - assert(qHi > qLo); > + assert(qHi >= qLo); > double pLo = quantile(qLo); > double pHi = quantile(qHi, (int)pLo); > return interBinMean(pLo, pHi); > -- > 2.39.2 >
diff --git a/src/ipa/rpi/controller/histogram.cpp b/src/ipa/rpi/controller/histogram.cpp index 0a27ba2c..78116141 100644 --- a/src/ipa/rpi/controller/histogram.cpp +++ b/src/ipa/rpi/controller/histogram.cpp @@ -47,7 +47,7 @@ double Histogram::quantile(double q, int first, int last) const double Histogram::interBinMean(double binLo, double binHi) const { - assert(binHi > binLo); + assert(binHi >= binLo); double sumBinFreq = 0, cumulFreq = 0; for (double binNext = floor(binLo) + 1.0; binNext <= ceil(binHi); binLo = binNext, binNext += 1.0) { @@ -57,13 +57,19 @@ double Histogram::interBinMean(double binLo, double binHi) const sumBinFreq += bin * freq; cumulFreq += freq; } + + if (cumulFreq == 0) { + /* interval had zero width or contained no weight? */ + return binHi; + } + /* add 0.5 to give an average for bin mid-points */ return sumBinFreq / cumulFreq + 0.5; } double Histogram::interQuantileMean(double qLo, double qHi) const { - assert(qHi > qLo); + assert(qHi >= qLo); double pLo = quantile(qLo); double pHi = quantile(qHi, (int)pLo); return interBinMean(pLo, pHi);
Don't assert when taking the weighted mean of a zero-width or zero-weight interval; return its upper bound. That is certainly correct in the zero-width case, and plausible otherwise. Signed-off-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com> --- src/ipa/rpi/controller/histogram.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)