[petsc-dev] Patches like this make me want to cry

Paul Mullowney paulm at txcorp.com
Thu Jun 28 19:03:39 CDT 2012


The TransplantPlaceArray, TransplantResetArray was very experimental and 
should have not have been committed. I removed it.




> This is minutia now, but this statement is nonsense (it triggers 
> -Waddress and is meaningless).
>
>    38.66 +  if (&y_array) y_array = PETSC_NULL;
>
> On Thu, Jun 28, 2012 at 2:53 PM, Jed Brown <jedbrown at mcs.anl.gov 
> <mailto:jedbrown at mcs.anl.gov>> wrote:
>
>     On Thu, Jun 28, 2012 at 2:41 PM, Paul Mullowney <paulm at txcorp.com
>     <mailto:paulm at txcorp.com>> wrote:
>
>         I moved all CUSPARSE functions into seqcusparse/aijcusparse.cu
>         <http://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
>         <http://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
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20120628/58543da2/attachment.html>


More information about the petsc-dev mailing list