[PATCH v2] test: ipa: rkisp1: utils: Fix floating and fixed point conversion test

[Kieran Bingham]

Quoting Kieran Bingham (2024-06-10 11:00:17)
> Quoting Paul Elder (2024-06-07 11:03:23)
> > There was an issue where using map to store the test cases meant that
> > the test for ignoring unused bits was skipped because of clashing keys.
> > Fix this by moving the offending test out of the loop.
> > 
> > While at it, also change the arbitrary floating comparison precision to
> > be more precise.
> > 
> > Also fix a missing documentation brief.
> > 
> > Fixes: 9d152e9c6 ipa: rkisp1: Add a helper to convert floating-point to
> > fixed-point
> 
> I think it's better to keep trailier lines on a single line even if it
> extends beyond 72/80 chars.
> 
> I think the title is normally in brackets and quotes too?
> 
> Fixes: 9d152e9c6 ("ipa: rkisp1: Add a helper to convert floating-point to fixed-point")
> 
> But just trivial nits. That doesn't seem to be enforced by checkstyle
> presently.

Hrm ... I take that back - checkstyle certainly does have a rule for
this:



commit_regex = re.compile(r'[0-9a-f]{12}[0-9a-f]* \(".*"\)')

    known_trailers = {
    	...
        'Fixes': commit_regex,
	...
    }

So - it should be enforced to be:

Fixes: 9d152e9c66c1 ("ipa: rkisp1: Add a helper to convert floating-point to fixed-point")

--
Kieran


> 
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> 
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > 
> > ---
> > Changes in v2:
> > - remove the added complexity that would've made it clearner to add a
> >   larger variety of tests because the variety wasn't there
> > ---
> >  test/ipa/rkisp1/rkisp1-utils.cpp | 24 ++++++++++++++++++++++--
> >  1 file changed, 22 insertions(+), 2 deletions(-)
> > 
> > diff --git a/test/ipa/rkisp1/rkisp1-utils.cpp b/test/ipa/rkisp1/rkisp1-utils.cpp
> > index e48f8d362..b1863894c 100644
> > --- a/test/ipa/rkisp1/rkisp1-utils.cpp
> > +++ b/test/ipa/rkisp1/rkisp1-utils.cpp
> > @@ -21,7 +21,23 @@ using namespace ipa::rkisp1;
> >  class RkISP1UtilsTest : public Test
> >  {
> >  protected:
> > -       template<unsigned int IntPrec, unsigned FracPrec, typename T>
> > +       /* R for real, I for integer */
> > +       template<unsigned int IntPrec, unsigned int FracPrec, typename I, typename R>
> > +       int testFixedToFloat(I input, R expected)
> > +       {
> > +               R out = utils::fixedToFloatingPoint<IntPrec, FracPrec, R>(input);
> > +               R prec = 1.0 / (1 << FracPrec);
> > +               if (std::abs(out - expected) > prec) {
> > +                       cerr << "Reverse conversion expected " << input
> > +                            << " to convert to " << expected
> > +                            << ", got " << out << std::endl;
> > +                       return TestFail;
> > +               }
> > +
> > +               return TestPass;
> > +       }
> > +
> > +       template<unsigned int IntPrec, unsigned int FracPrec, typename T>
> >         int testSingleFixedPoint(double input, T expected)
> >         {
> >                 T ret = utils::floatingToFixedPoint<IntPrec, FracPrec, T>(input);
> > @@ -54,7 +70,6 @@ protected:
> >                  */
> >                 std::map<double, uint16_t> testCases = {
> >                         { 7.992, 0x3ff },
> > -                       { 7.992, 0xbff },
> >                         {   0.2, 0x01a },
> >                         {  -0.2, 0x7e6 },
> >                         {  -0.8, 0x79a },
> > @@ -72,6 +87,11 @@ protected:
> >                                 return ret;
> >                 }
> >  
> > +               /* Special case with a superfluous one in the unused bits */
> > +               ret = testFixedToFloat<4, 7, uint16_t, double>(0xbff, 7.992);
> > +               if (ret != TestPass)
> > +                       return ret;
> > +
> >                 return TestPass;
> >         }
> >  
> > -- 
> > 2.39.2
> >