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

[Jed Brown]

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> wrote:

> On Thu, Jun 28, 2012 at 2:41 PM, Paul Mullowney <paulm at 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
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20120628/c3e7cc6f/attachment.html>