[vsipl++] lu solver

[Jules Bergmann]

> 
> Some nit-picking: If we insist on having a @date key in the files, they 
> should contain some real value, not just
> a copy of a file this originally was a copy of. (Note that I'm indeed 
> not sure about the need for @date, nor
> most of the other keys. But that's for another discussion...)
> 

Right.  Let's make the keys be valid.

(Changing/removing the keys is not open for discussion :)


>> +    VSIP_IMPL_THROW(unimplemented(
>> +      "Lud_impl cvsip solver doesn't support this transformation"));
>>   }
>>
>>  
>>
> 
> Since the above exception would percolate up to the public API, I don't 
> think "Lud_impl cvsip solver" is the best
> name to give to the actual code. May be "cvsip LU solver backend" ?
> 

I agree.  How about "LU solver (CVSIP backend) does not implement this 
transformation".  Start with the user-level VSIPL++ object that failed, 
give some extra info that might be useful (the backend in this case), 
then give the error.


>> Index: solver-lu.cpp
>>  
>>
> 
> As we just relocated and renamed most files, please make sure to follow 
> the naming
> conventions. Use '_' instead of '-', and use lu.hpp, instead of 
> solver_hpp (and likewise
> for the cpp).

This file is an existing unit test in the tests subdirectory (which 
would be more obvious if the diff was taken from the top-level)  -- i.e. 
we can't beat up on Assem too much for the name I gave it way back when :)

>> +
>> #define VERBOSE       0
>>  
>>
> 
> This looks like debug code. Should that really go into the repository ?
> 
>> #define DO_ASSERT     1
>>  
>>
> 
> Same here. Additionally, why don't you use <cassert> instead
> (i.e. a noop in release mode, and a real test with potential abort()
> otherwise) ?

This is in a unit test, so debug code like this is OK.  That way when we 
write a new backend, debugging test failures is marginally easier.

The DO_ASSERT flag lets assertions be turned off, which IIRC was useful 
in debugging for getting passed the first error to see other errors.

Just to be clear, in unit tests, we should use 'test_assert', not 
'assert' from <cassert>.  This lets us run the test cases with the 
release mode flags (which include -DNDEBUG, which disables asserts). 
Inside the library, we should use 'assert' for the same reason.

					-- Jules

-- 
Jules Bergmann
CodeSourcery
jules at codesourcery.com
(650) 331-3385 x705