[Insight-developers] Re: Threaded dilate/erode

[Gaetan Lehmann]

I have just wrote a small test to verify everything works with several  
threads.
It is available at  
http://voxel.jouy.inra.fr/darcs/contrib-itk/binaryErodeDilate/

The test dilate an image with a kernel of radius 20 to make a large  
overlaping region for each thread, and impose to run 100 threads. The  
result is compared with the output of the same filter with just 1 thread.
I have run the test 100 fold without any failure.
Everything seems to work at it should :-)

To compare, when I had problem in my filters, with 10 threads and a kernel  
of radius 5, I had errors for nearly each run.

On Mon, 21 Nov 2005 10:31:26 +0100, Gaetan Lehmann  
<gaetan.lehmann at jouy.inra.fr> wrote:

>
> It should be easy to verify if there is a problem by writing a small  
> test where the number of thread is set to a high value (10 for example).  
> The filter I'm write had some problem with threads and this kind of test  
> show them very clearly.
>
> I modified the threaded version of Jerome to implement the erode filter  
> and to solve the problem with the requested region, but have not tested  
> it with several threads. If there is really a problem, we should go back  
> to the non threaded version, but we need to be sure the bug with the  
> requested region is fixed.
>
> Gaetan
>
>
> On Mon, 21 Nov 2005 05:14:35 +0100, SCHMID, Jerome  
> <jeromeschmid at surgery.cuhk.edu.hk> wrote:
>
>> Miller, James V (Research) wrote:
>>
>>> Jerome, Gaetan,
>>>
>>> We have integrated the threaded versions of the binary dilate/erode  
>>> into the
>> ITK repository. The use of the ProgressReporter needed to be changed  
>> slightly in
>> the multithreaded versions to be thread safe. One of the arguments to  
>> the
>> ProgressReporter is the threadId. Thread 0 is only one that outputs  
>> information
>> by the ProgressReporter. This argument was set to 0 for all threads,  
>> causing
>> some intermittent errors.
>>>
>> Hi Jim,
>>
>> Glad to see that you have interest on the filter implementation. BTW,  
>> just by
>> curiosity, what is the effect of :
>> "(void)ThreadId;" in the .txx code?
>>
>>>
>>> I question whether stage 3 is really thread safe. It looks like this  
>>> loop can
>> write to pixels that are outside the outputRegionForThread. This could  
>> result in
>> two threads writing to the same memory location at the same time. This  
>> could
>> result in corrupted memory and in some cases crashes. Can you verify  
>> whether or
>> not stage 3 could write to pixels that are outside the  
>> outputRegionForThread.
>>>
>>
>> Well I have a deeper look and it doesnt't seem thread-safe... However I  
>> am not
>> an expert in iterators/multi-threading so let's see the hazardous  
>> points:
>>
>> // Paint SE --> "( BORDER(X) (+) B )" Part:
>>
>> // the dangerous iterator ^_^
>> NeighborhoodIterator<OutputImageType> onit;
>> onit = NeighborhoodIterator<OutputImageType>( kernel.GetRadius(),  
>> output,
>> outputRegionForThread );
>>
>> ....
>>
>> // the devil part
>> bool bIsInBound;
>> for( itIndex = indexDifferenceSet.begin(); itIndex !=  
>> indexDifferenceSet.end();
>> ++itIndex )
>> {
>> onit.SetPixel( *itIndex, foregroundValue, bIsInBound );
>> }
>>
>> So the pb is: the neighborhoodIterator will write in is "neighboor  
>> band" that
>> will overlap the close region of an other thread. The pb is that we  
>> need this
>> writing if we want to have a correct result so it cannot be supressed.
>>
>>
>> The last part doesn't cause pb because the output iterator is in the
>> requestedRegionFroThread and the input iterator is in the
>> requestedRegionFroThread padded but in READ mode, which should not  
>> cause pb, isn'it?
>>
>> So How to deal with the previous pb?
>>
>> Well I don't know exactly, I am not a thread expert...:-( But I think a  
>> mutex/barrier
>> lock may become necessary only on the pixels belonging to the  
>> neighborhood that
>> are outside the requested region of the iterator. This may require a new
>> iterator writing...
>>
>> Another solution, maybe not so easy to implement, would be to record  
>> these
>> pixels to be writing and problematic and to store them in a structure  
>> without
>> performing the writing. These pixels can be identified before the  
>> writing. Then,
>> if itk offers this possibility, write these pixels in a *non* threaded  
>> way (AfterGenerateThreadedData()
>> function or similar).
>>
>>
>>>
>>> Minor point: ITK style guide requires that TABS be replaced with  
>>> spaces. It
>> would make integrating contributed code into ITK smoother if your  
>> submissions
>> used spaces for indentation instead of TABS.
>>>
>>
>> Yes you already told me but the threaded code was submitted a long time  
>> ago...:-)
>> BTW I do not submit often !
>>
>> Tell me your thinking and we can try to find the best way. BTW I am  
>> surprised
>> that there are no filter in itk with the same problematic...
>>
>> Best Regards,
>>
>> Jerome Schmid
>>
>> -----------------------------------
>> Jerome SCHMID
>> Project Manager/ Engineer
>> Augmented and Virtual Reality
>> MIS Centre
>> Prince of Wales Hospital
>> Chinese University Of Hong-Kong
>> -----------------------------------
>>
>
>
>



-- 
Gaëtan Lehmann
Biologie du Développement et de la Reproduction
INRA de Jouy-en-Josas (France)
tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
http://voxel.jouy.inra.fr