[{"id":29803,"web_url":"https://patchwork.libcamera.org/comment/29803/","msgid":"<171802040284.1489778.16446832750459924206@ping.linuxembedded.co.uk>","date":"2024-06-10T11:53:22","subject":"Re: [PATCH] ipa: libipa: vector: Add matrix-vector multiplication","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Paul Elder (2024-06-07 09:20:45)\n> Add an operation for multiplying a matrix with a vector.\n> \n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> \n> ---\n> Depends on v6 of \"ipa: libipa: Add Matrix class\" and v5 of \"ipa: libipa:\n> Add Vector class\"\n> ---\n>  src/ipa/libipa/vector.cpp | 12 ++++++++++++\n>  src/ipa/libipa/vector.h   | 22 ++++++++++++++++++++++\n>  2 files changed, 34 insertions(+)\n> \n> diff --git a/src/ipa/libipa/vector.cpp b/src/ipa/libipa/vector.cpp\n> index bdc3c447c..e18426fe5 100644\n> --- a/src/ipa/libipa/vector.cpp\n> +++ b/src/ipa/libipa/vector.cpp\n> @@ -145,6 +145,18 @@ namespace ipa {\n>   * \\return The length of the vector\n>   */\n>  \n> +/**\n> + * \\fn Vector<T, R1> operator*(const Matrix<T, R1, C1> &m, const Vector<T, R2> &v)\n> + * \\brief Multiply a matrix by a vector\n> + * \\tparam T Numerical type of the contents of the matrix and vector\n> + * \\tparam R1 The number of rows in the matrix\n> + * \\tparam C1 The number of colums in the matrix\n> + * \\tparam R2 The number of elements in the vector\n> + * \\param m The matrix\n> + * \\param v The vector\n> + * \\return Product of matrix \\a m and vector \\a v\n> + */\n> +\n>  /**\n>   * \\fn bool operator==(const Vector<T, R> &lhs, const Vector<T, R> &rhs)\n>   * \\brief Compare vectors for equality\n> diff --git a/src/ipa/libipa/vector.h b/src/ipa/libipa/vector.h\n> index 87aad28b5..934b44494 100644\n> --- a/src/ipa/libipa/vector.h\n> +++ b/src/ipa/libipa/vector.h\n> @@ -17,6 +17,8 @@\n>  \n>  #include \"libcamera/internal/yaml_parser.h\"\n>  \n> +#include \"matrix.h\"\n> +\n>  namespace libcamera {\n>  \n>  LOG_DECLARE_CATEGORY(Vector)\n> @@ -166,6 +168,26 @@ private:\n>         std::array<T, R> data_;\n>  };\n>  \n> +#ifndef __DOXYGEN__\n> +template<typename T,\n> +        unsigned int R1, unsigned int C1,\n> +        unsigned int R2,\n> +        std::enable_if_t<std::is_arithmetic_v<T> && C1 == R2> * = nullptr>\n> +#endif /* __DOXYGEN__ */\n> +Vector<T, R1> operator*(const Matrix<T, R1, C1> &m, const Vector<T, R2> &v)\n> +{\n> +       Vector<T, R1> result;\n> +\n\nDoes this need to ASSERT(C1 == R2); or such?\n\nOther than that it looks ok to me, but I haven't seen the usage yet (or\ntests :D)\n\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\n> +       for (unsigned int i = 0; i < R1; i++) {\n> +               T sum = 0;\n> +               for (unsigned int j = 0; j < C1; j++)\n> +                       sum += m[i][j] * v[j];\n> +               result[i] = sum;\n> +       }\n> +\n> +       return result;\n> +}\n> +\n>  #ifndef __DOXYGEN__\n>  template<typename T, unsigned int R,\n>          std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>\n> -- \n> 2.39.2\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BF689BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 10 Jun 2024 11:53:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E4610634D5;\n\tMon, 10 Jun 2024 13:53:27 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0AC40634D5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 10 Jun 2024 13:53:26 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 091BC66F;\n\tMon, 10 Jun 2024 13:53:13 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"K80b5b2N\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718020394;\n\tbh=Pws9X0WUcWkGTA/9r1ixHucjC4SllzwIhb08degNF6A=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=K80b5b2NhIrtgmZPoLJPxTYkXrvPSFYzMRpvrUtgZgO70Cq60UJnE0Z3U5GzMxG8I\n\tvteyHIHQ4jjUmae/LS/JEW5WuFmnybRS5H1YVj/CK9+8dhluVORURia7wFSNDHmAe9\n\t2Y6y7+yngXVq6yNLI7OmXRQ4fZTjlwUsUosC9x+s=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240607082045.2718555-1-paul.elder@ideasonboard.com>","References":"<20240607082045.2718555-1-paul.elder@ideasonboard.com>","Subject":"Re: [PATCH] ipa: libipa: vector: Add matrix-vector multiplication","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Paul Elder <paul.elder@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 10 Jun 2024 12:53:22 +0100","Message-ID":"<171802040284.1489778.16446832750459924206@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":29811,"web_url":"https://patchwork.libcamera.org/comment/29811/","msgid":"<ZmcMvTjhUl8XFVM8@pyrite.rasen.tech>","date":"2024-06-10T14:25:01","subject":"Re: [PATCH] ipa: libipa: vector: Add matrix-vector multiplication","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Mon, Jun 10, 2024 at 12:53:22PM +0100, Kieran Bingham wrote:\n> Quoting Paul Elder (2024-06-07 09:20:45)\n> > Add an operation for multiplying a matrix with a vector.\n> > \n> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > \n> > ---\n> > Depends on v6 of \"ipa: libipa: Add Matrix class\" and v5 of \"ipa: libipa:\n> > Add Vector class\"\n> > ---\n> >  src/ipa/libipa/vector.cpp | 12 ++++++++++++\n> >  src/ipa/libipa/vector.h   | 22 ++++++++++++++++++++++\n> >  2 files changed, 34 insertions(+)\n> > \n> > diff --git a/src/ipa/libipa/vector.cpp b/src/ipa/libipa/vector.cpp\n> > index bdc3c447c..e18426fe5 100644\n> > --- a/src/ipa/libipa/vector.cpp\n> > +++ b/src/ipa/libipa/vector.cpp\n> > @@ -145,6 +145,18 @@ namespace ipa {\n> >   * \\return The length of the vector\n> >   */\n> >  \n> > +/**\n> > + * \\fn Vector<T, R1> operator*(const Matrix<T, R1, C1> &m, const Vector<T, R2> &v)\n> > + * \\brief Multiply a matrix by a vector\n> > + * \\tparam T Numerical type of the contents of the matrix and vector\n> > + * \\tparam R1 The number of rows in the matrix\n> > + * \\tparam C1 The number of colums in the matrix\n> > + * \\tparam R2 The number of elements in the vector\n> > + * \\param m The matrix\n> > + * \\param v The vector\n> > + * \\return Product of matrix \\a m and vector \\a v\n> > + */\n> > +\n> >  /**\n> >   * \\fn bool operator==(const Vector<T, R> &lhs, const Vector<T, R> &rhs)\n> >   * \\brief Compare vectors for equality\n> > diff --git a/src/ipa/libipa/vector.h b/src/ipa/libipa/vector.h\n> > index 87aad28b5..934b44494 100644\n> > --- a/src/ipa/libipa/vector.h\n> > +++ b/src/ipa/libipa/vector.h\n> > @@ -17,6 +17,8 @@\n> >  \n> >  #include \"libcamera/internal/yaml_parser.h\"\n> >  \n> > +#include \"matrix.h\"\n> > +\n> >  namespace libcamera {\n> >  \n> >  LOG_DECLARE_CATEGORY(Vector)\n> > @@ -166,6 +168,26 @@ private:\n> >         std::array<T, R> data_;\n> >  };\n> >  \n> > +#ifndef __DOXYGEN__\n> > +template<typename T,\n> > +        unsigned int R1, unsigned int C1,\n> > +        unsigned int R2,\n> > +        std::enable_if_t<std::is_arithmetic_v<T> && C1 == R2> * = nullptr>\n> > +#endif /* __DOXYGEN__ */\n> > +Vector<T, R1> operator*(const Matrix<T, R1, C1> &m, const Vector<T, R2> &v)\n> > +{\n> > +       Vector<T, R1> result;\n> > +\n> \n> Does this need to ASSERT(C1 == R2); or such?\n\nIt's in SFINAE... is that not sufficient?\n\n> \n> Other than that it looks ok to me, but I haven't seen the usage yet (or\n> tests :D)\n> \n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\nThanks,\n\nPaul\n\n> \n> \n> > +       for (unsigned int i = 0; i < R1; i++) {\n> > +               T sum = 0;\n> > +               for (unsigned int j = 0; j < C1; j++)\n> > +                       sum += m[i][j] * v[j];\n> > +               result[i] = sum;\n> > +       }\n> > +\n> > +       return result;\n> > +}\n> > +\n> >  #ifndef __DOXYGEN__\n> >  template<typename T, unsigned int R,\n> >          std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>\n> > -- \n> > 2.39.2\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9A6A8C31E9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 10 Jun 2024 14:25:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CEE3B65462;\n\tMon, 10 Jun 2024 16:25:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AED75634D5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 10 Jun 2024 16:25:08 +0200 (CEST)","from pyrite.rasen.tech (h175-177-049-156.catv02.itscom.jp\n\t[175.177.49.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AA410397;\n\tMon, 10 Jun 2024 16:24:55 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"jcACmTnN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718029496;\n\tbh=WSvgIE/LdcNXRI6qQpjSZLiFHyIZfy5pPNDYYTmzILM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jcACmTnNLiBG8gSXU+McU3u2z31heVewUzVEahAYBB/txBFLmVVNT3p30RpZ4feCc\n\thZoWowfAB7DMCpnEnA4RJ8TbsG4eY1WFm7+Uglgur/R5l/oPp4O+v3u3hZ68r9mYUR\n\t9BHVyXMSN3tEpwls3vvmmlLjj/o1Xp1Z3Xap7lPo=","Date":"Mon, 10 Jun 2024 23:25:01 +0900","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] ipa: libipa: vector: Add matrix-vector multiplication","Message-ID":"<ZmcMvTjhUl8XFVM8@pyrite.rasen.tech>","References":"<20240607082045.2718555-1-paul.elder@ideasonboard.com>\n\t<171802040284.1489778.16446832750459924206@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<171802040284.1489778.16446832750459924206@ping.linuxembedded.co.uk>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":29815,"web_url":"https://patchwork.libcamera.org/comment/29815/","msgid":"<171803114844.2248009.4797082254119393773@ping.linuxembedded.co.uk>","date":"2024-06-10T14:52:28","subject":"Re: [PATCH] ipa: libipa: vector: Add matrix-vector multiplication","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Paul Elder (2024-06-10 15:25:01)\n> On Mon, Jun 10, 2024 at 12:53:22PM +0100, Kieran Bingham wrote:\n> > Quoting Paul Elder (2024-06-07 09:20:45)\n> > > Add an operation for multiplying a matrix with a vector.\n> > > \n> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > > \n> > > ---\n> > > Depends on v6 of \"ipa: libipa: Add Matrix class\" and v5 of \"ipa: libipa:\n> > > Add Vector class\"\n> > > ---\n> > >  src/ipa/libipa/vector.cpp | 12 ++++++++++++\n> > >  src/ipa/libipa/vector.h   | 22 ++++++++++++++++++++++\n> > >  2 files changed, 34 insertions(+)\n> > > \n> > > diff --git a/src/ipa/libipa/vector.cpp b/src/ipa/libipa/vector.cpp\n> > > index bdc3c447c..e18426fe5 100644\n> > > --- a/src/ipa/libipa/vector.cpp\n> > > +++ b/src/ipa/libipa/vector.cpp\n> > > @@ -145,6 +145,18 @@ namespace ipa {\n> > >   * \\return The length of the vector\n> > >   */\n> > >  \n> > > +/**\n> > > + * \\fn Vector<T, R1> operator*(const Matrix<T, R1, C1> &m, const Vector<T, R2> &v)\n> > > + * \\brief Multiply a matrix by a vector\n> > > + * \\tparam T Numerical type of the contents of the matrix and vector\n> > > + * \\tparam R1 The number of rows in the matrix\n> > > + * \\tparam C1 The number of colums in the matrix\n> > > + * \\tparam R2 The number of elements in the vector\n> > > + * \\param m The matrix\n> > > + * \\param v The vector\n> > > + * \\return Product of matrix \\a m and vector \\a v\n> > > + */\n> > > +\n> > >  /**\n> > >   * \\fn bool operator==(const Vector<T, R> &lhs, const Vector<T, R> &rhs)\n> > >   * \\brief Compare vectors for equality\n> > > diff --git a/src/ipa/libipa/vector.h b/src/ipa/libipa/vector.h\n> > > index 87aad28b5..934b44494 100644\n> > > --- a/src/ipa/libipa/vector.h\n> > > +++ b/src/ipa/libipa/vector.h\n> > > @@ -17,6 +17,8 @@\n> > >  \n> > >  #include \"libcamera/internal/yaml_parser.h\"\n> > >  \n> > > +#include \"matrix.h\"\n> > > +\n> > >  namespace libcamera {\n> > >  \n> > >  LOG_DECLARE_CATEGORY(Vector)\n> > > @@ -166,6 +168,26 @@ private:\n> > >         std::array<T, R> data_;\n> > >  };\n> > >  \n> > > +#ifndef __DOXYGEN__\n> > > +template<typename T,\n> > > +        unsigned int R1, unsigned int C1,\n> > > +        unsigned int R2,\n> > > +        std::enable_if_t<std::is_arithmetic_v<T> && C1 == R2> * = nullptr>\n> > > +#endif /* __DOXYGEN__ */\n> > > +Vector<T, R1> operator*(const Matrix<T, R1, C1> &m, const Vector<T, R2> &v)\n> > > +{\n> > > +       Vector<T, R1> result;\n> > > +\n> > \n> > Does this need to ASSERT(C1 == R2); or such?\n> \n> It's in SFINAE... is that not sufficient?\n\nAh, you mean this line above:\n\n       std::enable_if_t<std::is_arithmetic_v<T> && C1 == R2> * = nullptr>\n\nI missed that. If it provides the compile time guarantees, then sure -\neven better.\n\n--\nKieran\n\n\n> \n> > \n> > Other than that it looks ok to me, but I haven't seen the usage yet (or\n> > tests :D)\n> > \n> > \n> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> \n> Thanks,\n> \n> Paul\n> \n> > \n> > \n> > > +       for (unsigned int i = 0; i < R1; i++) {\n> > > +               T sum = 0;\n> > > +               for (unsigned int j = 0; j < C1; j++)\n> > > +                       sum += m[i][j] * v[j];\n> > > +               result[i] = sum;\n> > > +       }\n> > > +\n> > > +       return result;\n> > > +}\n> > > +\n> > >  #ifndef __DOXYGEN__\n> > >  template<typename T, unsigned int R,\n> > >          std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>\n> > > -- \n> > > 2.39.2\n> > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E84B8C31E9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 10 Jun 2024 14:52:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DE23865466;\n\tMon, 10 Jun 2024 16:52:32 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AD870634D5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 10 Jun 2024 16:52:31 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9FD4E397;\n\tMon, 10 Jun 2024 16:52:19 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"TWciXSJJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718031139;\n\tbh=DGBo/mhFPss1I4CR4bvWvuluX86wY/hTObNugmQEKEM=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=TWciXSJJiZMJLY5k34Mzcj6SJWfABtrbNoEhtAz2ED0+rjvrNTSpmBrHzOR3wCkWH\n\t6BYZFgkzxsD5cT473MNPI8vykXhwya2YzqnKrkLJv5LkXE6IwFUNfeaIcQfXcmFvS5\n\tzJrPKsfZVkAC0aPW72nGUjOFCpMGixe1Lea6fTRA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<ZmcMvTjhUl8XFVM8@pyrite.rasen.tech>","References":"<20240607082045.2718555-1-paul.elder@ideasonboard.com>\n\t<171802040284.1489778.16446832750459924206@ping.linuxembedded.co.uk>\n\t<ZmcMvTjhUl8XFVM8@pyrite.rasen.tech>","Subject":"Re: [PATCH] ipa: libipa: vector: Add matrix-vector multiplication","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","To":"Paul Elder <paul.elder@ideasonboard.com>","Date":"Mon, 10 Jun 2024 15:52:28 +0100","Message-ID":"<171803114844.2248009.4797082254119393773@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":29853,"web_url":"https://patchwork.libcamera.org/comment/29853/","msgid":"<20240611225509.GD15006@pendragon.ideasonboard.com>","date":"2024-06-11T22:55:09","subject":"Re: [PATCH] ipa: libipa: vector: Add matrix-vector multiplication","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Jun 10, 2024 at 03:52:28PM +0100, Kieran Bingham wrote:\n> Quoting Paul Elder (2024-06-10 15:25:01)\n> > On Mon, Jun 10, 2024 at 12:53:22PM +0100, Kieran Bingham wrote:\n> > > Quoting Paul Elder (2024-06-07 09:20:45)\n> > > > Add an operation for multiplying a matrix with a vector.\n> > > > \n> > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > > > \n> > > > ---\n> > > > Depends on v6 of \"ipa: libipa: Add Matrix class\" and v5 of \"ipa: libipa:\n> > > > Add Vector class\"\n> > > > ---\n> > > >  src/ipa/libipa/vector.cpp | 12 ++++++++++++\n> > > >  src/ipa/libipa/vector.h   | 22 ++++++++++++++++++++++\n> > > >  2 files changed, 34 insertions(+)\n> > > > \n> > > > diff --git a/src/ipa/libipa/vector.cpp b/src/ipa/libipa/vector.cpp\n> > > > index bdc3c447c..e18426fe5 100644\n> > > > --- a/src/ipa/libipa/vector.cpp\n> > > > +++ b/src/ipa/libipa/vector.cpp\n> > > > @@ -145,6 +145,18 @@ namespace ipa {\n> > > >   * \\return The length of the vector\n> > > >   */\n> > > >  \n> > > > +/**\n> > > > + * \\fn Vector<T, R1> operator*(const Matrix<T, R1, C1> &m, const Vector<T, R2> &v)\n> > > > + * \\brief Multiply a matrix by a vector\n> > > > + * \\tparam T Numerical type of the contents of the matrix and vector\n> > > > + * \\tparam R1 The number of rows in the matrix\n> > > > + * \\tparam C1 The number of colums in the matrix\n> > > > + * \\tparam R2 The number of elements in the vector\n> > > > + * \\param m The matrix\n> > > > + * \\param v The vector\n> > > > + * \\return Product of matrix \\a m and vector \\a v\n> > > > + */\n> > > > +\n> > > >  /**\n> > > >   * \\fn bool operator==(const Vector<T, R> &lhs, const Vector<T, R> &rhs)\n> > > >   * \\brief Compare vectors for equality\n> > > > diff --git a/src/ipa/libipa/vector.h b/src/ipa/libipa/vector.h\n> > > > index 87aad28b5..934b44494 100644\n> > > > --- a/src/ipa/libipa/vector.h\n> > > > +++ b/src/ipa/libipa/vector.h\n> > > > @@ -17,6 +17,8 @@\n> > > >  \n> > > >  #include \"libcamera/internal/yaml_parser.h\"\n> > > >  \n> > > > +#include \"matrix.h\"\n> > > > +\n> > > >  namespace libcamera {\n> > > >  \n> > > >  LOG_DECLARE_CATEGORY(Vector)\n> > > > @@ -166,6 +168,26 @@ private:\n> > > >         std::array<T, R> data_;\n> > > >  };\n> > > >  \n> > > > +#ifndef __DOXYGEN__\n> > > > +template<typename T,\n> > > > +        unsigned int R1, unsigned int C1,\n> > > > +        unsigned int R2,\n> > > > +        std::enable_if_t<std::is_arithmetic_v<T> && C1 == R2> * = nullptr>\n> > > > +#endif /* __DOXYGEN__ */\n> > > > +Vector<T, R1> operator*(const Matrix<T, R1, C1> &m, const Vector<T, R2> &v)\n\nI think you can simplify all this as\n\ntemplate<typename T, unsigned int Rows, unsigned int Cols>\nVector<T, Rows> operator*(const Matrix<T, Rows, Cols> &m, const Vector<T, Cols> &v)\n\n> > > > +{\n> > > > +       Vector<T, R1> result;\n> > > > +\n> > > \n> > > Does this need to ASSERT(C1 == R2); or such?\n> > \n> > It's in SFINAE... is that not sufficient?\n> \n> Ah, you mean this line above:\n> \n>        std::enable_if_t<std::is_arithmetic_v<T> && C1 == R2> * = nullptr>\n> \n> I missed that. If it provides the compile time guarantees, then sure -\n> even better.\n> \n> > > Other than that it looks ok to me, but I haven't seen the usage yet (or\n> > > tests :D)\n\nIs it used in any series already posted that I have missed, or should I\njust wait ?\n\nI think we need unit tests for this (as well as the Vector and Matrix\nclasses). We don't have to go overboard and test every trivial functions\nin the initial implementation. Not a big blocker, but it should be\nrecorded somewhere and added.\n\n> > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > >\n> > > > +       for (unsigned int i = 0; i < R1; i++) {\n> > > > +               T sum = 0;\n> > > > +               for (unsigned int j = 0; j < C1; j++)\n> > > > +                       sum += m[i][j] * v[j];\n> > > > +               result[i] = sum;\n> > > > +       }\n> > > > +\n> > > > +       return result;\n> > > > +}\n> > > > +\n> > > >  #ifndef __DOXYGEN__\n> > > >  template<typename T, unsigned int R,\n> > > >          std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 430C5BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 11 Jun 2024 22:55:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6C74365461;\n\tWed, 12 Jun 2024 00:55:31 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0865261A26\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 12 Jun 2024 00:55:30 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B4505230;\n\tWed, 12 Jun 2024 00:55:16 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"GQw1WgDj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718146516;\n\tbh=WlAyBx9pJFFlgFxZlqyPvcvzB3r/w2A6GPxi0bX92ms=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GQw1WgDjJe3gAHL0ZanRQYDmALZxnElobTOaoXy7/zMDG9s6HSJM467/s6xlzmJ7f\n\tgOOTvGEFiUOSvkPGfZ+A0lMwAHMPXaYsp5Yf2B8AyCOnGRKNzi1B1AhxOTSUGUfjYf\n\tFU6i9am/e91Cr38uXZcEZnNPerhaJ53m/cj+p6H0=","Date":"Wed, 12 Jun 2024 01:55:09 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] ipa: libipa: vector: Add matrix-vector multiplication","Message-ID":"<20240611225509.GD15006@pendragon.ideasonboard.com>","References":"<20240607082045.2718555-1-paul.elder@ideasonboard.com>\n\t<171802040284.1489778.16446832750459924206@ping.linuxembedded.co.uk>\n\t<ZmcMvTjhUl8XFVM8@pyrite.rasen.tech>\n\t<171803114844.2248009.4797082254119393773@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<171803114844.2248009.4797082254119393773@ping.linuxembedded.co.uk>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]