[Insight-developers] Re: Threaded dilate/erode

[Miller, James V (Research)]

Gaetan, 

My experience with these issues says that a given filter may only fail 
ONCE every 1000 or 10000 times. It all depends on the size of the image, size
of the neighborhood, and most importantly, the load on the system.

So seeing a thread problem in the code, I suspect that it is only a matter 
of time before the presents itself.

Jim

-----Original Message-----
From: Gaetan Lehmann [mailto:gaetan.lehmann at jouy.inra.fr]
Sent: Monday, November 21, 2005 5:22 AM
To: SCHMID, Jerome; Miller, James V (Research); Insight-developers
(E-mail)
Subject: Re: [Insight-developers] Re: Threaded dilate/erode



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