[Insight-developers] Very suspicious behavior.

[Luis Ibanez]

Hans,

On Mon, Feb 28, 2011 at 2:34 PM, Johnson, Hans J <hans-johnson at uiowa.edu> wrote:
> Luis,
>
> I disagree.
>
> 1) In ITKv3 this behavior only occurred when USE_OPTIMIZED_REGISTRATION
> was turned on, in the other case the behavior was as I expected it.


Of course,

The metrics, are only multi-threaded when
 USE_OPTIMIZED_REGISTRATION is ON

(this in fact should have been called
"MULTI_THREADED_REGISTRATION"
instead of "OPTIMIZED_REGISTRATION"...)


> 2) The only part of the registration framework that I want single threaded
> is the metric part,  any other part of the multi-threader should continue
> to multithreaded if possible.


There are no other multi-threaded pieces
in the registration framework.


> 3) I assumed that a call to SetNumberOfThreads() only affects the object
> and it's parent objects, not the ivars of the class.
>

The RegistrationMethod *is* a filter,

When we call SetNumberOfThreads() in a filter,
we expect the behavior of this filter to be such
that it uses the number of threads that we requested.


> My program allows the end user to choose the metric at run-time.  I only
> want the metric to be single threaded if it is MattesMutualInformation.
> I'd have to dynamically cast my metric object to determine if it is a
> MattesMutualInformation type, and then set the number of threads only in
> that case.
>

Not necessarily.

If your user decided to use MattesMI, there is logic in your application
(the logic one that interacts with the user's request) that know when
MattesMI has been requested.  At that level there is no need of
dynamic casting.


You have changed the behavior of ITK
to accommodate a convenience of your application.


> Additionally, the RegistrationFramework does the setting of the number of
> threads inside of the Initialization routine.  If the behavior is to be as
> you describe, it should be decoupled from the initialization routine.
>


Not at all,

The Initialize() method is called inside of the StartRegistration() method,
that is equivalent to Update().

If you set the number of Threads before calling Update() the logic in
the Initialize() method will comply to that request.

The numberOfThreads is an ivar in the ProcessObject, the parent
of the ImageRegistrationMethod class.


> Please let me know if you still dis-agree with me.  It will require a ton
> of extra programming to work around this behavior.
>

Yeap.
I still think that that line shouldn't be removed.



>
> ==========
> In the particular case in question, I set the
> metric->SetNumberOfThreads(1);
> I did NOT set the number of threads for the RegistrationMethod.
>

You should have set the number of threads in the registration method.


> The behavior was that the metric was run with 4 threads.  That seems very
> wrong to me.
>

Not if you consider that the Metric is a helper to the registration.

I also set the initial set of parameters to the Transform itself, and
those values change immediately after the registration starts, due
to the fact that the RegistrationMethod is the one orchestrating the
process.


> The registration frame work may be a process object, but it only
> coordinates other objects.  If someone wrote an interpolator that was
> multi-threaded, what would the expected behavior be?
>

I would expect it to be consistent with the setting of the
filter (which is the registration method in this case).



     Luis




--------------------------------------------
> Thanks,
>
> Hans
>
>
>
>
> On 2/28/11 1:05 PM, "Luis Ibanez" <luis.ibanez at kitware.com> wrote:
>
>>Hi Hans,
>>
>>This is not quite a bug.
>>
>>It is intended behavior based on the fact that the RegistrationMethod,
>>not the Metric, is the one that has a Filter API.
>>
>>If you want to control the number of threads, you can put the setting
>>in the registration method.
>>
>>                       registration->SetNumberOfThreads( 1 );
>>
>>Just as you would do with any multi-threaded filter.
>>
>>
>>    Luis
>>
>>
>>---------------------------------------------------
>>On Mon, Feb 28, 2011 at 8:09 AM, Johnson, Hans J <hans-johnson at uiowa.edu>
>>wrote:
>>> Dan,
>>> I found a tangentially related bug in ITKv3 & ITKv4 that was
>>>complicating my
>>> understanding of the BRAINSFit threading issues that you are so kindly
>>> looking into.
>>> Rather than make the entire program single threaded, I wanted to
>>>isolate and
>>> force just itk::MattesMutualInformationImageToImageMetric  into single
>>> threaded mode:
>>> BRAINSCommonLib/BRAINSFitHelper.cxx :  Inserted line 115:
>>> 106   if(this->m_CostMetric == "MMI")
>>> 107     {
>>> 108     typedef
>>>
>>>itk::MattesMutualInformationImageToImageMetric<FixedVolumeType,MovingVolu
>>>meType>
>>> MetricType;
>>> 109     this->SetupRegistration<MetricType>();
>>> 110
>>> 111     MetricType::Pointer localCostMetric =
>>> this->GetCostMetric<MetricType>();
>>> 112
>>>
>>>localCostMetric->SetNumberOfHistogramBins(this->m_NumberOfHistogramBins);
>>> 113     const bool UseCachingOfBSplineWeights = (
>>> m_UseCachingOfBSplineWeightsMode == "ON" ) ? true : false;
>>> 114
>>>
>>>localCostMetric->SetUseCachingOfBSplineWeights(UseCachingOfBSplineWeights
>>>);
>>> 115     localCostMetric->SetNumberOfThreads(1);
>>> 116     this->RunRegistration<MetricType>();
>>> 117     }
>>> The problem was the that ImageRegistrationMethod was modifying the
>>>state of
>>> my localCostMetric and resetting the number of threads to a larger
>>>value.
>>> I've posted patches to both ITKv3 and ITKv4 to avoid changing the
>>> prescribed localCostMetric number of threads:
>>> http://review.source.kitware.com/#change,1088
>>> http://review.source.kitware.com/#change,1087
>>> =============================================
>>> Once the ITKv[3,4] bugs are fixed, then I can sacrifice time to get a
>>> consistent result in BRAINSFit ( by forcing localCostMetric for MMI to
>>>only
>>> use 1 thread).
>>> =============================================
>>> I hope you can find a way to give consistent threaded results from "MMI"
>>> because that greatly increases the processing time when it is single
>>> threaded.
>>> Thanks,
>>> Hans
>>>
>>>
>>> ________________________________
>>> Notice: This UI Health Care e-mail (including attachments) is covered
>>>by the
>>> Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is
>>>confidential
>>> and may be legally privileged.  If you are not the intended recipient,
>>>you
>>> are hereby notified that any retention, dissemination, distribution, or
>>> copying of this communication is strictly prohibited.  Please reply to
>>>the
>>> sender that you have received the message in error, then delete it.
>>>Thank
>>> you.
>>> ________________________________
>>> _______________________________________________
>>> Powered by www.kitware.com
>>>
>>> Visit other Kitware open-source projects at
>>> http://www.kitware.com/opensource/opensource.html
>>>
>>> Kitware offers ITK Training Courses, for more information visit:
>>> http://kitware.com/products/protraining.html
>>>
>>> Please keep messages on-topic and check the ITK FAQ at:
>>> http://www.itk.org/Wiki/ITK_FAQ
>>>
>>> Follow this link to subscribe/unsubscribe:
>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>
>>>
>
>
>
> ________________________________
> Notice: This UI Health Care e-mail (including attachments) is covered by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential and may be legally privileged.  If you are not the intended recipient, you are hereby notified that any retention, dissemination, distribution, or copying of this communication is strictly prohibited.  Please reply to the sender that you have received the message in error, then delete it.  Thank you.
> ________________________________
>