From: Karl R. <ru...@iu...> - 2014-05-22 08:29:17
|
Hey, > Shame on me. This is not even the first time that something like this > happens to me. I hope I'll never forget to enable this flag anymore! it'll save you lot of headache :-) > Also sorry for the coding style... I'll fix it. Let's change this in a coordinated fashion next week. I think it's better to do this once throughout the whole library rather than incrementally throwing random commits at it. We have both lots of code that doesn't comply, so if we don't fix this together, we will just change one inconsistency into another ;-) Best regards, Karli > 2014-05-22 10:20 GMT+02:00 Karl Rupp <ru...@iu... > <mailto:ru...@iu...>>: > > Hi again, > > the problem was 'just' a missing return statement, through which the > returned vector wasn't created properly. I pushed a fix already, and > another commit which adds '-Wextra' to the list of compilation flags > if ENABLE_PEDANTIC is set. It is not enabled by default in order not > to shy away users who clone our developer repository, yet I hope > that everybody intending to commit code to the repository will have > the flag enabled. (Exception: When building CUDA code, -Wextra is > hopeless, as nvcc produces tons of warnings in the generated code) > > Best regards, > Karli > > > > On 05/22/2014 01:12 AM, Philippe Tillet wrote: > > I'm just realizing that, while such checks make sense in the case of > self-assignement, we probably shouldn't do it for inner_prod(), > since it > should technically result in a different expression tree. The > good way > to implement it would be through a pre-processor for > scheduler::statement (like in Theano), but we're not there yet. So, > i'll just perform an additional check within CG for now > > Philippe > > > 2014-05-21 23:04 GMT+02:00 Philippe Tillet > <phi...@gm... <mailto:phi...@gm...> > <mailto:phi...@gm... <mailto:phi...@gm...>>__>: > > > Hi, > > Ok thanks, i'll do it! > I did this trick: > > template<typename VectorType, typename > PreconditionerType> > class z_handler{ > public: > z_handler(VectorType & residual) : z_(residual){ } > VectorType & get() { return z_; } > private: > VectorType z_; > }; > > template<typename VectorType> > class z_handler<VectorType, > viennacl::linalg::no_precond>{ > public: > z_handler(VectorType & residual) : > presidual_(&residual){ } > VectorType & get() { return *presidual_; } > private: > VectorType * presidual_; > }; > > And then, in the code: > > z_handler<VectorType, PreconditionnerType> zh(residual); > VectorType& z = zh.get(); > > The rest can either be done as if(&z == &residual) or by > checking > self-assignments, and the address of the arguments of norm. > I've > chosen the latter option, since I don't see any drawback to > redirecting inner_prod to norm2 when the two arguments are > the same, > or returning *this directly for a self-assignment. Plus, > some users > might end up doing similar things too, so it's better to > have it > handled internally > > Philippe > > > > 2014-05-21 21:35 GMT+02:00 Karl Rupp <ru...@iu... > <mailto:ru...@iu...> > <mailto:ru...@iu... > <mailto:ru...@iu...>>__>: > > > Hi, > > > > I've slightly modified the CG implementation to handle > preconditionned > > CG and unpreconditionned CG in the same routine > (without > adding any > extra allocation/copy), but I can't find a routine > to test > that I > haven't introduced a bug. Running > grep "cg_tag" tests/src/* > returns no result. > Do we have a test for the iterative solvers, or > just examples? > > > unfortunately there is no native test in ViennaCL > itself. Some > other Vienna-libraries test their solvers against > viennacl-dev, > so this part of functionality is checked indirectly. > > The easiest way to verify proper operation 'by hand' is > to check > the residuals reported through 'solverbench'. > > Btw: How does your unified implementation behave > performance-wise? I think the reason this was not > unified is > that a plain CG somewhere saves a memory transfer (norm() > instead of inner_prod() or related). On the other hand, the > pipelined CG version I'll push soon will require some extra > tweaks for ViennaCL-types, so it's good to have a single > 'generic' implementation for non-ViennaCL types. > > Best regards, > Karli > > > > > |