[Insight-developers] Very suspicious behavior.

[Johnson, Hans J]

OK.  I'm not convinced, but I'll concede.

Then in ITKv4 there needs to be MUCH better documentation.  It took me
almost 3 hours to figure out why the metric kept reverting back to a
different number of threads even though I had explicitly demanded that it
only use 1 thread.  In my mind the request for number of threads is
conceptually much different that setting the initial value of a transform.

I also think it is confusing to have delegated the responsibility for
setting the number of threads to the registration framework.
Perhaps in the RegistrationFramework we should change the name to
"SetMetricNumberOfThreads" to indicate that this just passes the request
on to the current metric, and only after the RegistrationFramework is
initialized.

================

We should not have backwards compatible issues because this behavior was
only visible in ITKv3 with Review filters.

Thanks,
Hans



On 2/28/11 2:06 PM, "Luis Ibanez" <luis.ibanez at kitware.com> wrote:

>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,MovingVo
>>>>lu
>>>>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(UseCachingOfBSplineWeigh
>>>>ts
>>>>);
>>>> 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.
>> ________________________________
>>



________________________________
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.
________________________________