On Thu, Jun 28, 2012 at 2:41 PM, Paul Mullowney <paulm@txcorp.com> wrote:
I moved all CUSPARSE functions into seqcusparse/aijcusparse.cu (and a similar mpi version). I was asked to do this some months ago. I did my best. I think it is significantly improved from before.

I was also asked by multiple people to attempt to get Complex GPU capabilities working. I succeeded by making changes to petscmath.h. As Jed pointed out, some of the changes may have not been C99 compliant.

It needs to work with C89, not just C99. Global changes to handle complex should also be done in separate patches.

I hate the idea that CUSP leaks out into user code so I would recommend casting internally rather than changing it globally. That should also enable use of complex without requiring the user to adopt a C++ compiler for their own code (C99 _Complex is binary-compatible with std::complex and cusp::complex).
 

There is one commented out function in aijcusparse.cu. That can easily be fixed.

Many of the other changes (especially to .cu) files were made to protect against builds breaking because many of the CUSP preconditioners do not support complex arithmetic.

Those should absolutely be in separate patches. Lumping it all into one makes it impossible to review or test.
 

I have asked for input from multiple people on several occasions and not gotten responses.

What am I to do if I ask for input/feedback and don't get it?????

Next time, please provide the patch series. You could use "hg email" or post your branch to bitbucket or elsewhere so that we can comment.

Also, please follow the coding guidelines. There is a developer's manual and frequent discussion on petsc-dev about conventions. (If you notice something that is adopted, but not in the developer's manual, please point it out on the mailing list or make a patch.)
 


-Paul




There are essentially no comments on this massive push. I was just reading it, getting mad. Jed mailed first.
Pushes need enough comments that I can see exactly what functionality is being added and how it works.
You will not be the maintainer of this functionality, we will. If we can't do it, it will be thrown out, and all your
time will be wasted.

   Matt
 

This is non-portable:

http://petsc.cs.iit.edu/petsc/petsc-dev/rev/66ca8db0d5f8


Can we please institute some sort of policy on patch quality/reviewability? This one patch is going to take a significant amount of fix-up (not made easier by the several merges since) and/or generate several build failures and user inconvenience (petsc-maints). No doubt the functionality is important, but we just don't have time to fix these things line-by-line after they are pushed.





--
What most experimenters take for granted before they begin their experiments is infinitely more interesting than any results to which their experiments lead.
-- Norbert Wiener