[Insight-developers] ITK 3.12: FREEZING TODAY
Luis Ibanez
luis.ibanez at kitware.com
Thu Feb 26 11:23:35 EST 2009
Hi Luca,
Thanks for the clarifications.
At this point, we probably don't want to make changes to this code
unless they can be qualified as "bug fixes".
I agree with your suggestion for using a specific ScaleImageType.
It makes a lot of sense.
Given that the class is in the Code/Review directory, we can still
improve its API after the release.
If we attempt to make changes to templated classes today, it will
still take a couple of Nightly dashboard cycles to verify such
changes on all platforms. :-/
At this point we probably should focus on any remaning issues in
the Dashboard, that could be consider show stoppers for the Release.
Luis
---------------------
Luca Antiga wrote:
> Bill, Luis,
> you're both right, in the sense that, yes, the EigenValueArrayType uses
> the dimension of the pixel type, which happens to be indeed equal to the
> image type for the typical use case. So I expect the code to run correctly.
>
> I'm thinking about cases where this assumption would not hold... like
> when one computes the 2D Hessian (say in the xy plane) on 3D image.
>
> Although one could argue that this is unlikely (and untested!), I think
> we have to make a decision here. From the point of view of keeping
> concepts clean (which always pays off in the long run), I'd go back to
> using InputPixelType::Dimension, but the other way is also fine (the
> assumption should be documented, though).
>
>
> As to the warning about the "short unsigned int" to "double" warning (by
> the way, thanks for all the work you're doing around my filter!): the
> cast fix solves the problem, but it introduces a potential problem in
> the functionality.
> OutputImageType can be defined with an integer PixelType, which will
> result in an integer type enhanced image. The problem is that the
> maximum response scale image should report the exact scale (float type)
> at which the filter responded the best, without converting it to integer
> type, since this would lead to wrong information. In other words, the
> scales image always has to be float (I wouldn't use double for this,
> there's no use).
>
> Summarizing, I would introduce a
> typedef float ScalesPixelType
> typdef Image<ScalesPixelType> ScalesImageType;
> and change the rest of the class to use ScalesImageType instead of
> OutputImageType for output 1.
>
> I don't know if you want to keep the class in for 3.12, so just let me
> know if I can commit changes to it (or if you want to do it).
>
> Best
>
>
> Luca
>
>
> On Feb 25, 2009, at 9:33 PM, Luis Ibanez wrote:
>
>>
>> Bill, Luca,
>>
>> Yes, that piece was a bit confusing.
>>
>> My reasoning was:
>>
>> The type is used to define the Eigen values of the Hessian,
>> that happens to be the input image type.
>>
>> Therefore the length of the array that will store the eigenvalues
>> must match the dimension of the image.
>>
>> Typically the pixel type of a Hessian Image will be a symmetric
>> second rank tensor, in which case the "Dimension" still refers to
>> the dimension of the space in which the image is embedded.
>>
>>
>> Luca:
>> Is this consistent with the intention you had in this code ?
>>
>>
>>
>> Thanks for any hint,
>>
>>
>> Luis
>>
>>
>> ------------------------------------------------------------------------------------------------
>> On Wed, Feb 25, 2009 at 3:04 PM, Bill Lorensen
>> <bill.lorensen at gmail.com <mailto:bill.lorensen at gmail.com>> wrote:
>>
>> Luis,
>>
>> I think these changes:
>> http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/Review/itkHessianToObjectnessMeasureImageFilter.h?root=Insight&r1=1.1&r2=1.2&sortby=date
>> <http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/Review/itkHessianToObjectnessMeasureImageFilter.h?root=Insight&r1=1.1&r2=1.2&sortby=date>
>>
>> do not match the original intent. The EigenValueArrayType used
>> InputPixelType::Dimension. The changes are the equivalent of using
>> InputImageType::ImageDimension. The dimension is of the pixeltype not
>> the imagetype.
>>
>> Bill
>>
>> On Wed, Feb 25, 2009 at 2:07 PM, Luis Ibanez
>> <luis.ibanez at kitware.com <mailto:luis.ibanez at kitware.com>> wrote:
>> >
>> > Hi Bill,
>> >
>> > We are addressing the issues in Borland and VS6...
>> > but if it start taking too long, we can simply remove the new files.
>> >
>> > I can't wait to be able to install Visual Studio 6.0 in a computer
>> > in the Amazon cloud... :-)
>> >
>> >
>> > Luis
>> >
>> >
>> >
>> ----------------------------------------------------------------------
>> > On Wed, Feb 25, 2009 at 12:17 PM, Bill Lorensen
>> <bill.lorensen at gmail.com <mailto:bill.lorensen at gmail.com>>
>> > wrote:
>> >>
>> >> Are we comfortable freezing if code was just checked into Review
>> >> yesterday? I suggest we remove the new stuff that was checked
>> in for
>> >> now, even though it is in Review. Borland as well as VS6 has
>> >> problems. I made sure before I went away to have a clean VS6 build.
>> >>
>> >> Bill
>> >>
>> >> On Wed, Feb 25, 2009 at 11:59 AM, Bill Lorensen
>> <bill.lorensen at gmail.com <mailto:bill.lorensen at gmail.com>>
>> >> wrote:
>> >> > You will also need to use itkGetStaticConstMacro for the
>> static const
>> >> > that is declared in that header. This is standard practice for
>> >> > Borland.
>> >> >
>> >> > I have restarted my continuous for a couple of days.
>> >> >
>> >> > Bill
>> >> >
>> >> > On Wed, Feb 25, 2009 at 11:55 AM, Luis Ibanez
>> <luis.ibanez at kitware.com <mailto:luis.ibanez at kitware.com>>
>> >> > wrote:
>> >> >> Hi Bill,
>> >> >>
>> >> >> Yeap, we are tracking those.
>> >> >>
>> >> >> They seem to be related to the direct use of
>> ImageType::Dimension.
>> >> >> We are replacing it with the dimension declaration extracted
>> with the
>> >> >> GetImageDimension helper class.
>> >> >>
>> >> >>
>> >> >> Luis
>> >> >>
>> >> >>
>> >> >> ---------------------------------------------------
>> >> >> On Wed, Feb 25, 2009 at 11:40 AM, Bill Lorensen
>> >> >> <bill.lorensen at gmail.com <mailto:bill.lorensen at gmail.com>>
>> >> >> wrote:
>> >> >>>
>> >> >>> Recent checkins to Review have broken the Borland build.
>> This should
>> >> >>> be repaired. I cannot help at this moment.
>> >> >>>
>> >> >>> On Wed, Feb 25, 2009 at 10:34 AM, Bradley Lowekamp
>> >> >>> <blowekamp at mail.nih.gov <mailto:blowekamp at mail.nih.gov>> wrote:
>> >> >>> > I am building an experimental with the fix for this (and the
>> >> >>> > ImageIORegion
>> >> >>> > M-D to N-D conversion issues):
>> >> >>> >
>> >> >>> > http://public.kitware.com/Bug/view.php?id=8369
>> >> >>> > After that I am good :)
>> >> >>> >
>> >> >>> > On Feb 25, 2009, at 10:04 AM, Luis Ibanez wrote:
>> >> >>> >
>> >> >>> > Unless anyone has a strong objection, we will be freezing
>> >> >>> > the cvs repository today in order to clean up the last
>> >> >>> > details before we cut the release of ITK 3.12 on Saturday.
>> >> >>> >
>> >> >>> > Objections anyone ?
>> >> >>> >
>> >> >>> >
>> >> >>> > Thanks
>> >> >>> >
>> >> >>> >
>> >> >>> > Luis
>> >> >>> >
>> >> >>> > _______________________________________________
>> >> >>> > Powered by www.kitware.com <http://www.kitware.com>
>> >> >>> >
>> >> >>> > Visit other Kitware open-source projects at
>> >> >>> > http://www.kitware.com/opensource/opensource.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
>> >> >>> >
>> >> >>> >
>> >> >>> > _______________________________________________
>> >> >>> > Powered by www.kitware.com <http://www.kitware.com>
>> >> >>> >
>> >> >>> > Visit other Kitware open-source projects at
>> >> >>> > http://www.kitware.com/opensource/opensource.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
>> >> >>> >
>> >> >>> >
>> >> >>
>> >> >>
>> >> >
>> >
>> >
>>
>>
>> _______________________________________________
>> Powered by www.kitware.com <http://www.kitware.com>
>>
>> Visit other Kitware open-source projects at
>> http://www.kitware.com/opensource/opensource.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
>
>
> --
> Luca Antiga, PhD
> Head, Medical Imaging Unit,
> Biomedical Engineering Department,
> Mario Negri Institute.
> mail: Villa Camozzi, 24020, Ranica (BG), Italy
> phone: +39 035 4535-381
> email: antiga at marionegri.it <mailto:antiga at marionegri.it>
> web: http://villacamozzi.marionegri.it/~luca
>
More information about the Insight-developers
mailing list