Re: [eigen] patch for tridiagonal matrices in eigen |

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

*To*: eigen@xxxxxxxxxxxxxxxxxxx*Subject*: Re: [eigen] patch for tridiagonal matrices in eigen*From*: "Benoit Jacob" <jacob.benoit.1@xxxxxxxxx>*Date*: Mon, 15 Dec 2008 07:04:00 -0500*Dkim-signature*: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from:to :subject:in-reply-to:mime-version:content-type :content-transfer-encoding:content-disposition:references; bh=9XGj9/fhqaLEu4e6+ggrF6R5WtWQgcWVBdH+3IvPivY=; b=G6G66e2JIOfWHNXH773yDb4LEUaYQ3HfJFLc2Emgv85RB/VgTHO+YZjJt3gLe6xWeT cHYliBnezSwX7AoI3J23nEk9tdRnzcjvXwZtFjFXDyJqiLVssSBKpTvhz5WohkU9+c+9 b7NuPSwkhwAyeOaP3+VzGabWFafHpAsv0lpZI=*Domainkey-signature*: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:to:subject:in-reply-to:mime-version :content-type:content-transfer-encoding:content-disposition :references; b=YmbaI2CcAfZMJlf5oy8yPwflxFYGVoS2mao7eTt1ORqOV1bC9rqdfR2Y6ZyTE2iDLi 3Q3KWkCz+5yvo2rciJxbCuQKNwJ9FTvzqTymQNuhWlFRt1HW4f/vuetbdH2V3U5X9eAW OrorPzys1vXbgsguyIl+Ova7Byg/ai9ON0WKc=

2008/12/15 Mauro Iazzi <mauro.iazzi@xxxxxxxxx>: >I adapt them to the correct coding style (do you have guidelines written somewhere?). No, but if you quickly read through the files MatrixBase.h and Assign.h for example you get an idea of the style, then we're not too maniac. > I meant to define a new matrix class and specializing expressions, > like product. It was very easy to do, so it's already very extensible, > but I had to change the headers for that. Ideally I could just define > without having to put new code in the library. For example, as you saw, the logic choosing the product mode (ei_product_mode) is in Product.h so you'd always have to change that. The only way to do that from outside Eigen would be by partial template specialization but that would basically require to know beforehand which extra template parameters to do the specialization on... not very convenient. That said I hear your point, there's always room for improvement for better extensibility. >> -const unsigned int Upper = UpperTriangularBit; >> -const unsigned int StrictlyUpper = UpperTriangularBit | ZeroDiagBit; >> -const unsigned int Lower = LowerTriangularBit; >> -const unsigned int StrictlyLower = LowerTriangularBit | ZeroDiagBit; >> +const unsigned int Upper = UpperTriangularBit | UpperHessenbergBit; >> +const unsigned int StrictlyUpper = Upper | ZeroDiagBit; >> +const unsigned int Lower = LowerTriangularBit | LowerHessenbergBit; >> +const unsigned int StrictlyLower = Lower | ZeroDiagBit; >> >> here I don't really follow. These are parameters for part(). So unless >> you want to add Hessenberg support in part (which would be a good >> idea) no need to change that. If you want to add Hessenberg support, >> you need to make the corresponding changes in Part.h. > > I saw that the product helper checked for diagonals using the flags, > so I did the same for tridiagonals. OK right, sorry. These are always convenient shortcuts. What needs to be updated then is the comment saying that these are the parameters for part(), actually they're not used only for part(). >> Here, the compiler will have to go through the whole logic of BOTH >> cases, which means uselessly long compilation times. Yes the compiler >> will always optimize the "if" itself (even in debug mode) so it's not >> a runtime speed issue, nor a code size issue, it's really a >> compilation-time issue, and we're extremely careful about that. > > understood. Maybe switching to MatrixType::SquareMatrixType also > inside Tridiagonalization<> would allow to specialize it for the > tridiagonal case. This would avoid storing and checking the flags and > also the manual copy of the vectors, since we could use the ones that > are already there (m_diagonalCoeffs, etc...). I'd need to have a closer look at the code in order to be able to comment on that.... (can't now) >> And, the line >> if ( (Flags & Tridiagonal) == Tridiagonal ) { >> can be written as just >> if(Flags & Tridiagonal) { > > I think that would incorrectly get Upper/Lower Hessenberg as > Tridiagonals... I've seen (Flags & Diagonal) == Diagonal in other > places for the same reason. Oh right, nevermind. I didn't remember that diagonal (and tridiag) were more than one bit... we probably need a little comment when we do that, if ( (Flags & Tridiagonal) == Tridiagonal ) // Tridiagonal is two bits and same thing for what we already do with diagonal... Last note: it seems that you're already a kde contributor... so you'll commit yourself. Cheers, Benoit > > cheers, > > mauro > > --- > > ---

**Follow-Ups**:**Re: [eigen] patch for tridiagonal matrices in eigen***From:*Benoit Jacob

**References**:**[eigen] patch for tridiagonal matrices in eigen***From:*Mauro Iazzi

**Re: [eigen] patch for tridiagonal matrices in eigen***From:*Benoit Jacob

**Re: [eigen] patch for tridiagonal matrices in eigen***From:*Mauro Iazzi

**Messages sorted by:**[ date | thread ]- Prev by Date:
**Re: [eigen] patch for tridiagonal matrices in eigen** - Next by Date:
**Re: [eigen] Usage of Eigen2 in a closed source application** - Previous by thread:
**Re: [eigen] patch for tridiagonal matrices in eigen** - Next by thread:
**Re: [eigen] patch for tridiagonal matrices in eigen**

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