[libcamera-devel] [PATCH v3 01/12] utils: tuning: libtuning: Implement the core of libtuning

[David Plowman]

Hi Paul

Thanks for doing all this.

On Wed, 23 Nov 2022 at 10:56, Paul Elder <paul.elder at ideasonboard.com> wrote:
>
> Hi David,
>
> <snip>
>
> > > > > diff --git a/utils/tuning/libtuning/utils.py b/utils/tuning/libtuning/utils.py
> > > > > new file mode 100644
> > > > > index 00000000..8a9f13f7
> > > > > --- /dev/null
> > > > > +++ b/utils/tuning/libtuning/utils.py
> > > > > @@ -0,0 +1,152 @@
> > > > > +# SPDX-License-Identifier: BSD-2-Clause
> > > > > +#
> > > > > +# Copyright (C) 2019, Raspberry Pi Ltd
> > > > > +# Copyright (C) 2022, Paul Elder <paul.elder at ideasonboard.com>
> > > > > +#
> > > > > +# utils.py - Utilities for libtuning
> > > > > +
> > > > > +import decimal
> > > > > +import math
> > > > > +import numpy as np
> > > > > +import os
> > > > > +from pathlib import Path
> > > > > +import re
> > > > > +import sys
> > > > > +
> > > > > +import libtuning as lt
> > > > > +from libtuning.image import Image
> > > > > +from libtuning.macbeth import locate_macbeth
> > > > > +
> > > > > +# Utility functions
> > > > > +
> > > > > +
> > > > > +def eprint(*args, **kwargs):
> > > > > +    print(*args, file=sys.stderr, **kwargs)
> > > > > +
> > > > > +
> > > > > +def get_module_by_type_name(modules, name):
> > > > > +    for module in modules:
> > > > > +        if module.type == name:
> > > > > +            return module
> > > > > +    return None
> > > > > +
> > > > > +
> > > > > +# @brief Round value while keeping the maximum number of decimal points
> > > > > +# @param limits Tuple of [min, max] acceptable values
> > > > > +# @description Prevents rounding such that significant figures are lost
> > > > > +# \todo Bikeshed this name
> > > > > +def round_with_sigfigs(val, limits: tuple):
> > > > > +    decimal_points = abs(decimal.Decimal(str(limits[-1])).as_tuple().exponent)
> > > >
> > > > To be honest, I wonder if deducing the decimal point from the limits is
> > > > worth it. For all you know, you may have a [0.0, 4.0] range and want 3
> > > > decimal points. I'd rather pass the precision to the function.
> > >
> > > Given the two sample points that I have I didn't think that you'd have a
> > > range of [0.0, 4.0].
> > >
> > > This means we'll have to add a new module parameter for precision. Which
> > > I guess is fine; range + precision.
> > >
> > > > > +
> > > > > +    # These are decimal left-shift multipliers
> > > > > +    lshift = 10**(decimal_points - 1)
> > > > > +    adjust = 10**(-decimal_points)
> > > > > +
> > > > > +    # We need the division to get rid of stray floating points
> > > > > +    # These are bounds for 5% and 95% of one significant figure *lower* than
> > > > > +    # the maximum number. They allow checking if a normal rounding would cause
> > > > > +    # an "overflow rounding" (where significant decimal points would be lost).
> > > > > +    # The "overflow rounding" can then be prevented by adding or subtracting
> > > > > +    # adjust.
> > > > > +    lower_bound = adjust * 10 * 5 * lshift / lshift
> > > > > +    upper_bound = adjust * 10 * 95 * lshift / lshift
> > > > > +
> > > > > +    out = val
> > > > > +    out = np.where((lshift * out) % 1 <= lower_bound, out + adjust, out)
> > > > > +    out = np.where((lshift * out) % 1 >= upper_bound, out - adjust, out)
> > > > > +    out = np.round(out, 3)
> > > >
> > > > You write in a reply to v2
> > > >
> > > > > "Round value while keeping the maximum number of decimal points"
> > > > > So like if limits is [0, 3.999], then 2.5999 would normally get rounded
> > > > > to 2.6, but this function would make sure it gets rounded to 2.599.
> > > >
> > > > Why is that desired ? The rounding error is larger.
> > >
> > > Good question. I don't know the answer. I just maintaned behavior from ctt.
> >
> > Maybe it was bad behaviour to start with ? :-)
>
> Do you have any insight on this? I rewrote for generic precision based
> on what's in ctt_alsc.py:
>
>             t_r = np.mean(list_cr[indices], axis=0)
>             t_b = np.mean(list_cb[indices], axis=0)
>             """
>             force numbers to be stored to 3dp.... :(
>             """
>             t_r = np.where((100*t_r) % 1 <= 0.05, t_r+0.001, t_r)
>             t_b = np.where((100*t_b) % 1 <= 0.05, t_b+0.001, t_b)
>             t_r = np.where((100*t_r) % 1 >= 0.95, t_r-0.001, t_r)
>             t_b = np.where((100*t_b) % 1 >= 0.95, t_b-0.001, t_b)
>             t_r = np.round(t_r, 3)
>             t_b = np.round(t_b, 3)
>
> But as Laurent points out above, this could end up with larger rounding
> error. What's the idea behind this?

I'm afraid I don't know. I wonder a little bit if it was done so that
they'd align nicely when you print them out, but of course there are
other ways of doing that too. However, for the sake of +/- 0.001 I
wouldn't worry very much. In fact I don't think I would worry at all,
the errors in tables being "not quite right", or the calibration
illumination being "not quite the same" are way bigger factors!

David

>
>
> Thanks,
>
> Paul