Re: [eigen] Enhanced AlignedBox |

[ Thread Index | Date Index | More lists.tuxfamily.org/eigen Archives ]

*To*: eigen@xxxxxxxxxxxxxxxxxxx*Subject*: Re: [eigen] Enhanced AlignedBox*From*: Gael Guennebaud <gael.guennebaud@xxxxxxxxx>*Date*: Tue, 22 Dec 2009 14:48:51 +0100*Dkim-signature*: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:message-id:subject:from:to:content-type; bh=GY9ZcamD89RAp203N7nThP3ve/esE2V+E7WlW8JTnlQ=; b=xb7e4LOmmAW20C74P0ZmHTOleDpvsZht/noTujca3JYlzWyJ4HETWnavi/QqYJLdoy rFbuvVqNB5u12LjGvO+gZUIhoMPa0QUosIp5Ff1lXkFduw7/iT7jhYnSIdK0rwZx8Ykl frEPcnXP5RHoZ2o8vd/sI6pnIH75dbI764Z5Y=*Domainkey-signature*: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; b=DOKn07V3PLQmsx2xeYUq3lBFe/jFbdvzVmWgkSpy+03i+noYRR4RjyxYxd7h8welYe dxWbFQI1sNoFaiU8+FDzosarmbV3tqUhnJB1nrK439eFdAZCsNhgF4BnLKGugT8rb5Wl Rl30+QFlDXlQ6dWdWzTtlMMd2QrT2+5vG67Bc=

Hi Manuel, thanks for the update, but I still have a few remarks (I forgot some in my previous email): * for the implementation of sizes you can simply do: return m_max - m_min; that is much simpler. If you wonder, yes, the type of "m_max-m_min" is "CwiseBinaryOp< ei_scalar_difference_op<Scalar>, VectorType, VectorType>". * for center(), in order to have zero temporary you have to nest multiple expressions like this: inline const CwiseUnaryOp<ei_scalar_quotient1_op<Scalar>, CwiseBinaryOp< ei_scalar_sum_op<Scalar>, VectorType, VectorType> > center() const { return (m_min + m_max)/2; } * Why did you template the functions taking another aligned box. (e.g., contains(), extend(), clamp(), etc.). I mean, both the dimension and the scalar type have to be exactly the same, so no need to template them. * Likewise you templated the functions initially taking a VectorType that is a good thing to avoid necessary copies when the actual parameter is not exactly the same type (e.g., a Block expression). However you must honor the "Nested" type of the expression. Let me explain on this example: template<typename Derived> inline bool contains(const MatrixBase<Derived>& p) const { return (m_min.cwise()<=p).all() && (p.cwise()<=m_max).all(); } Here "p" is used twice. So for instance if someone do: MyAABB aabb(XX,YY); aabb.contains((a+b)/2); since the type of "(a+b)/2" is an expression of the sum of two vectors divided by two, this expression will be evaluated twice by the function contains that is not very good. This might be even worse if the expression is more complicated like a product, or the solution of a solver, e.g.: aabb.contains(A.lu().solve(b)); The equation Ax=b will be solved twice! So the solution is to copy each of such generic arguments to their respective ideal nesting type according to the number of evaluation, e.g.: template<typename Derived> inline bool contains(const MatrixBase<Derived>& a_p) const { const typename ei_nested<Derived,2>::type p(a_p.derived()); return (m_min.cwise()<=p).all() && (p.cwise()<=m_max).all(); } Here the "2" is because p is going to be evaluated twice. Eigen will compare the cost of evaluating a_p twice to the cost of evaluating it once to a temporary and then access twice to the temporary, and according to the result of this comparison, typename ei_nested<Derived,2>::type will be either a reference "Derived&" or, in this case, the same as "VectorType". side note: Now I'm also thinking that this expression is not as efficient as it could be because it would be more efficient to loop through p only once using a ternary operators like: p.cwise().isInRange(m_min,m_max).all(); but that's not currently possible. Note that such an optimization would only makes sense for large dimensions. * finally it would be cool if you could configure your editor to use two spaces instead of "tabs" for the indentation gael On Mon, Dec 21, 2009 at 9:10 PM, Manuel Yguel <manuel.yguel@xxxxxxxxx> wrote: > Hi Gael, > thanks for the useful remarks. > I agree with all you said and I modify the code according to what you suggested. > > I am not at all familiar with how the expressions work in Eigen, so > consider it a try. > It seems to me that there is probably a simplest way to write the > code, so correct me if there is one: I will learn. > > I am thinking of the same kind of design that the one you suggested > for the random number generator. > I like the interface of the boost random number generators, so I will > try to make a proposition compatible with those ones. > > > - best regards, > > Manuel >

**Follow-Ups**:**Re: [eigen] Enhanced AlignedBox***From:*Manuel Yguel

**References**:**[eigen] Enhanced AlignedBox***From:*Manuel Yguel

**Re: [eigen] Enhanced AlignedBox***From:*Gael Guennebaud

**Re: [eigen] Enhanced AlignedBox***From:*Manuel Yguel

**Messages sorted by:**[ date | thread ]- Prev by Date:
**Re: [eigen] Enhanced AlignedBox** - Next by Date:
**Re: [eigen] Enhanced AlignedBox** - Previous by thread:
**Re: [eigen] Enhanced AlignedBox** - Next by thread:
**Re: [eigen] Enhanced AlignedBox**

Mail converted by MHonArc 2.6.19+ | http://listengine.tuxfamily.org/ |