[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