I concur with Brad and Hans. Adding Gaëtan in CC.
Thanks, Matt On Tue, Jul 2, 2013 at 5:08 PM, Johnson, Hans J <[email protected]> wrote: > I agree with brad. The two should produce the same results, even tough it > may introduce a different numerical result in the > OtsuMultipleThresholdCalculator. > > -----Original Message----- > From: Bradley Lowekamp <[email protected]> > Date: Tuesday, July 2, 2013 12:01 PM > To: "Padfield, Dirk R (GE Global Research)" <[email protected]> > Cc: ITK <[email protected]>, "[email protected]" > <[email protected]> > Subject: Re: [Insight-developers] OtsuThresholdCalculator > versus OtsuMultipleThresholdsCalculator > > Dirk, > > I would vote to have the two produce the same results, and create a little > migration guide which notes the change. > > Regarding, the refactoring, is there any difference in the algorithm > complexity of the two? Any measurements of the performance difference > between the two? > > Brad > > On Jul 2, 2013, at 11:04 AM, "Padfield, Dirk R (GE Global Research)" > <[email protected]> wrote: > >> Hi Richard, >> >> Thank you for your response and insight. If anything would need to be >>changed, I would also lean towards changing the OtsuMultiple because I >>think we shouldn't change Otsu since I am sure many more people are using >>the Otsu because it is a standard thresholding algorithm. I will do some >>comparisons of the two against other implementations to see what I find. >> >> But the question still remains: is it okay to change the OtsuMultiple to >>give the same output as Otsu for one threshold? I am thinking in terms >>of those people who use OtsuMultiple whose results will then be slightly >>different. Here are the advantages and disadvantages for keeping things >>the same: >> >> Advantages: everyone's code still works as it did before >> Disadvantages: the two implementations are inconsistent with each other >>even though they are the same algorithm. The overlapping code cannot be >>merged (inheritance). And future enhancements will need to be made in >>both places. >> >> My vote is: change OtsuMultiple to be consistent with Otsu. >> >> What do others think? >> >> Dirk >> >> >> ________________________________ >> From: Richard Beare [[email protected]] >> Sent: Monday, July 01, 2013 5:40 PM >> To: Bradley Lowekamp >> Cc: Padfield, Dirk R (GE Global Research); <[email protected]> >>Developers >> Subject: Re: [Insight-developers] OtsuThresholdCalculator versus >>OtsuMultipleThresholdsCalculator >> >> Hi, >> I think the order was - I introduced new filters copied from ImageJ, >>then Gaetan started refactoring to use the histogram framework. We both >>did some work to make that correspond to old versions. I don't remember >>working on the MultipleThreshold version, but the code does look >>similar, so perhaps it was done somewhere along the way - will need to >>check the logs. >> >> I'm pretty sure that Otsu was producing the same results that it used to >>- I didn't compare to other implementations. Thus, if the original Otsu >>was correct then the current one should be too, which would suggest that >>the MultipleThresholds version should probably change. >> >> Not sure when I'll get a chance to look at this in detail. >> >> I don't have a current email for Gaetan to CC for confirmation. >> >> >> On Mon, Jul 1, 2013 at 11:02 PM, Bradley Lowekamp >><[email protected]<mailto:[email protected]>> wrote: >> Dirk, >> >> I believe Richard Beare did the refactoring of the thresholding >>framework an Insight Journal Article. He will likely know why it is this >>way better than anyone else. >> >> You also didn't say which implementation is correct. >> >> Brad >> >> >> On Jun 30, 2013, at 10:03 PM, "Padfield, Dirk R (GE Global Research)" >><[email protected]<mailto:[email protected]>> wrote: >> >>> Hi ITK Developers, >>> >>> I was just looking through the OtsuThresholdCalculator and >>>OtsuMultipleThresholdsCalculator to see whether I could refactor them so >>>that the Otsu inherits from the OtsuMultiple since the latter is a more >>>general case of the former. Currently, the code for these two filters >>>is totally different resulting in significant code duplication and a >>>need to keep both filters in sync. >>> >>> As a first step, I wrote a CMake test to check that the output of the >>>OtsuMultiple with 1 threshold is the same as the output of the Otsu. >>>Unfortunately, they are not! The two filters output thresholds that are >>>different by 1 histogram bin! This can be a quite extreme difference >>>when the numberOfHistogramBins is low, and it leads to different >>>thresholds even when the numberOfHistogramBins is reasonably high (say >>>256). I tracked it down to this code in the Calculators: >>> >>> The relevant code from Otsu: >>> const double tolerance = 0.00001; >>> if ( (varBetween - tolerance) > maxVarBetween ) >>> { >>> maxVarBetween = varBetween; >>> maxBinNumber = j; >>> } >>> } >>> this->GetOutput()->Set( static_cast<OutputType>( >>>histogram->GetMeasurement( maxBinNumber + 1, 0 ) ) ); >>> >>> The relevant code from MultipleOtsu: >>> if ( varBetween > maxVarBetween ) >>> { >>> maxVarBetween = varBetween; >>> maxVarThresholdIndexes = thresholdIndexes; >>> } >>> } >>> for ( j = 0; j < m_NumberOfThresholds; j++ ) >>> { >>> m_Output[j] = histogram->GetBinMax(0, maxVarThresholdIndexes[j]); >>> } >>> >>> The difference is that the Otsu adds one to the computed threshold >>>whereas the MultipleOtsu does not. This is problematic because users >>>would expect them to give the same result. >>> >>> My question is: how should we proceed? If we change one or the other, >>>people's code that use the changed one will give slightly different >>>answers. If we don't change them, the two filters will give different >>>outputs for the same input, and it will not be possible to refactor them >>>to share code. >>> >>> What are your thoughts? >>> >>> Thanks, >>> Dirk >>> _______________________________________________ >>> Powered by www.kitware.com<http://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.php >>> >>> 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 >> >> > > _______________________________________________ > 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.php > > 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. > ________________________________ > _______________________________________________ > 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.php > > 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 _______________________________________________ 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.php 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
