[Insight-developers] GPU code ready for Gerrit

[Xiaoxiao Liu]

Hi Kris,
Thanks for the details.
I think it makes sense to have the GPU modules in different groups
according to their functionality. The only thing I am concerned is that we
lose the convenience of having a Group switch for GPU modules. But I guess
we can use ITK_USE_GPU  to do something similar.

What I did earlier with  5 video modules is that I checked in the least
dependent module(VideoCore) for review first and gradually checked the rest
of modules in bundles according to the module dependencies. I wouldn't
trying to put all modules to review at once. If you check in a giant patch,
it might take longer to review than the patch by patch way. Anyway, just my
two cents.

I could help you to set up a couple of dashboard testing machines. At least
you could submit experimental builds from your machine to make sure
reviewers could verify the patches.

-Xiaoxiao




On Thu, Feb 2, 2012 at 1:46 PM, Kris Zygmunt <krismz at sci.utah.edu> wrote:

> Hi Xiaoxiao,
>     Basically what I have done is taken Won-Ki's code, made sure it passes
> tests on multiple platforms, cleaned up warnings, and then done some cmake
> fixes to convert the gpu kernels to strings.
>
> >> Extra comments:
> I saw the GPU modules are in Core group. How about  making a GPU group? So
> that later gpu modules could naturally located in the same group folder?
>
>   I have also changed the gpu directory structure.  Originally, everything
> was in Modules/GPU/Common, I have changed it to the following where there
> is a corresponding GPU folder for each module that has both a CPU and GPU
> implementation:
>
> Modules
>       Core
>           Common
>           GPUCommon
>            ...
>       Filtering
>            Smoothing
>            GPUSmoothing
>             Thresholding
>             GPUThresholding
>              ...
>        ...
>
> I have performed a series of commits while working from my github <https://github.com/krismz/ITK><https://github.com/krismz/ITK>
>> https://github.com/krismz/ITK  GPU-Beta-R1 branch, is it worth saving
>> these as a series of commits when pushing to gerrit or is it better to
>> squash them into a single commit and lose some of the details of the
>> changes made?  If I push a series of commits to gerrit, what is the correct
>> way to do that to keep from creating multiple gerrit topics?
>
> >> It is the best to make small changes from the very beginning to avoid
> the giant-commit problem. But I guess it is too late now.  If you don't
> mind losing your development history in ITK history, you could do the
> single commit; If not, there are some git magic (norecommended, but Brad
> King knows how ) that  may help you to merge in the commit history. But
> either way, you might want to send the giant patch to gerrit for review
> first .  Looks like your github version of ITK is synced with the main ITK
> repo, which is good.  How about sending patches to gerrit by modules ? One
> GPU module as one topic  commit?
>
> I think it could be hard to provide a gerrit patch for each module
> separately and maintain the commit history of the changes I made.  The main
> reason I worked out of github was to get the GPU code mature enough for
> inclusion without disrupting the ITK 4 RC schedule.  So I suppose it comes
> down to asking what is preferred, making 7 different gerrit patches where
> each patch is one GPU module (each module has between 1 and 29 files in it)
> and losing any intermediate commit history or providing one large patch
> with all of the GPU code together?  I am concerned that providing a series
> of smaller patches will complicate things as each module except for
> Core/GPUCommon is dependent on other GPU modules, so ther would be
> complicated structure of linked gerrit patches to manage through the review
> process.  I think it would also be harder to manage STYLE changes across
> multiple gerrit patches.  Anyway, just my 2 cents, I will do whatever is
> preferred to help these changes get reviewed and merged quickly.
>
> Also, You probably want to set up at least one dashboard machine with GPU
> for the nightly dashboard.
>
> Can this machine be set up to use the gerrit code, or should it wait until
> after the code is merged in?  The nightly dashboard machine will need to
> have OpenCL installed and a relatively modern GPU card.  Then the only
> extra thing is to configure cmake with ITK_USE_GPU turned ON.  Can someone
> help configure one of the existing dashboard machines for this?
>
> -Kris
>
>


-- 


---------------------------------------------
*Xiaoxiao Liu*, Ph.D.
R & D Engineer
Kitware Inc <http://www.kitware.com/>.
Clifton Park, NY
Phone: (518) 881-4924  or  (518) 371-3971 x124
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20120202/1e0e32b4/attachment.htm>