[libcamera-devel] [PATCH 2/6] utils: libtuning: modules: lsc: Add debug plot function

[Dan Scally]

Hi Paul - thanks for the set, sorry for the delayed review...this is a really cool feature! I like 
it a lot.

On 24/11/2022 11:38, Paul Elder wrote:
> Add a helper function to the base LSC class for plotting an LSC table.
> This will be used by LSC specializations for debugging pruposes.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>   utils/tuning/libtuning/modules/lsc/lsc.py | 31 +++++++++++++++++++++++
>   1 file changed, 31 insertions(+)
>
> diff --git a/utils/tuning/libtuning/modules/lsc/lsc.py b/utils/tuning/libtuning/modules/lsc/lsc.py
> index f54ca8be..4d580ad4 100644
> --- a/utils/tuning/libtuning/modules/lsc/lsc.py
> +++ b/utils/tuning/libtuning/modules/lsc/lsc.py
> @@ -8,7 +8,10 @@ from ..module import Module
>   import libtuning as lt
>   import libtuning.utils as utils
>   
> +import matplotlib.pyplot as plt
> +from matplotlib import cm


Is it time to get a requirements.txt going for libtuning maybe?

>   import numpy as np
> +from pathlib import Path
>   
>   
>   class LSC(Module):
> @@ -67,3 +70,31 @@ class LSC(Module):
>               table = table / np.min(table)
>   
>           return table, grid
> +
> +    def _plot_single_lsc(self, image: lt.Image, cr: np.array, cb: np.array,
> +                         cg1: np.array, cg2: np.array = None):

Typed python still looks weird to me, though I appreciate it more nowadays. If you're going to type 
the parameters though I think the return value should be typed, and the function needs a docstring 
(I think particularly mentioning that it's suitable for RGB plots as well as RGGB (or whatever).


> +        hf = plt.figure(figsize=(8, 8))
> +        # Y is plotted as -Y so plot has same axes as image
> +        X, Y = np.meshgrid(range(self.sector_shape[0]), range(self.sector_shape[1]))
> +
> +        ha = hf.add_subplot(221 if cg2 is not None else 311, projection='3d')
> +        ha.plot_surface(X, -Y, cr, cmap=cm.coolwarm, linewidth=0)
> +        ha.set_title(f'{self.hr_name} Plot\nImage: {image.name}\n\ncr')


If I'm honest I don't think the 3D plot is particularly useful; I'd prefer it was just 2D and rely 
on the colour map to show the relative strength of the gains. In that vein I've personally been 
treating them as a heatmap and plotting with imshow(cr, interpolation='nearest'), though there might 
be a better way.

> +
> +        hb = hf.add_subplot(222 if cg2 is not None else 312, projection='3d')
> +        hb.plot_surface(X, -Y, cb, cmap=cm.coolwarm, linewidth=0)
> +        hb.set_title('cb')
> +
> +        hc = hf.add_subplot(223 if cg2 is not None else 313, projection='3d')
> +        hc.plot_surface(X, -Y, cg1, cmap=cm.coolwarm, linewidth=0)
> +        hc.set_title('gr' if cg2 is not None else 'g')
> +
> +        if cg2 is not None:
> +            hd = hf.add_subplot(224, projection='3d')
> +            hd.plot_surface(X, -Y, cg2, cmap=cm.coolwarm, linewidth=0)
> +            hd.set_title('gb')
> +
> +        # The caller will validate self.debug (there's no sense in calling this
> +        # function if self.debug is None anyway)
> +        fname = self.debug / f"{image.name.split('.')[0]}-lsc.png"
> +        plt.savefig(fname)