<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
Hi Amy,<br>
<br>
<pre wrap="">Thanks a lot for pointing these errors, and for all your suggestions to improve this code!
</pre>
<blockquote type="cite" style="color: rgb(0, 0, 0);">
<blockquote type="cite" style="color: rgb(0, 0, 0);">
<blockquote type="cite" style="color: rgb(0, 0, 0);">
<pre wrap="">- itkAtanRegularizedHeavisideStepFunction.h - in
<span class="moz-txt-citetags"></span>CalculateDerivative:
<span class="moz-txt-citetags"></span>derivative should be 1/pi*1/epsilon*1/(1+(x/epsilon)<sup
class="moz-txt-sup">2</sup>), not
<span class="moz-txt-citetags"></span>1/pi*1/(1+(x/epsilon)<sup
class="moz-txt-sup">2</sup>) (most people use epsilon==1 so there may
<span class="moz-txt-citetags"></span>be no
<span class="moz-txt-citetags"></span>noticable difference)
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre wrap="">/** Evaluate the derivative at the specified input position */
virtual OutputType EvaluateDerivative( const InputType& input ) const
{
const RealType t = ( input * this->GetOneOverEpsilon() );
return static_cast< OutputType>( vnl_math::one_over_pi / (1.0 + t * t ) );
}
</pre>
<blockquote type="cite" style="color: rgb(0, 0, 0);">
<blockquote type="cite" style="color: rgb(0, 0, 0);">
<blockquote type="cite" style="color: rgb(0, 0, 0);">
<pre wrap=""><span class="moz-txt-citetags">>>> </span>itkRegionBasedLevelSetFunction.txx - in ComputeGlobalTerm:
</pre>
</blockquote>
</blockquote>
</blockquote>
ok! Done<br>
<a class="moz-txt-link-freetext" href="http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/Review/itkRegionBasedLevelSetFunction.txx?root=Insight&r1=1.28&r2=1.29">http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/Review/itkRegionBasedLevelSetFunction.txx?root=Insight&r1=1.28&r2=1.29</a><br>
<blockquote type="cite" style="color: rgb(0, 0, 0);">
<blockquote type="cite" style="color: rgb(0, 0, 0);">
<blockquote type="cite" style="color: rgb(0, 0, 0);">
<pre wrap=""><span class="moz-txt-citetags">>>> </span>itkMultiphaseDenseFiniteDifferenceImageFilter
</pre>
</blockquote>
</blockquote>
</blockquote>
ok! Done<br>
I have also added accessors to m_ReinitializeCounter (Set/Get).<br>
<a class="moz-txt-link-freetext" href="http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/Review/itkMultiphaseDenseFiniteDifferenceImageFilter.h?root=Insight&r1=1.4&r2=1.5">http://public.kitware.com/cgi-bin/viewcvs.cgi/Code/Review/itkMultiphaseDenseFiniteDifferenceImageFilter.h?root=Insight&r1=1.4&r2=1.5</a><br>
<br>
<blockquote type="cite" style="color: rgb(0, 0, 0);">
<blockquote type="cite" style="color: rgb(0, 0, 0);">
<blockquote type="cite" style="color: rgb(0, 0, 0);">
<pre wrap=""><span class="moz-txt-citetags">>>> </span>possibly want to make ComputeHessian virtual in
<span class="moz-txt-citetags">>>> </span>itkRegionBasedLevelSetFunction.h
</pre>
</blockquote>
</blockquote>
</blockquote>
<pre wrap="">ok! Done (Kishore is about to commit it)
It should appear pretty soon on the dashboard
Thanks,
Cheers
Arnaud
</pre>
<br>
<br>
<br>
On 2/23/2010 5:53 PM, Kishore Mosaliganti wrote:
<blockquote
cite="mid:cd51098d1002231453x2ae51887sbca6d8601a2dfa56@mail.gmail.com"
type="cite">
<pre wrap="">Hi Amy and Luis,
Thank you very much for reviewing the code and providing the feedback.
This will certainly improve the code quality.
I think the corrections can happen in the Code/Review quite easily. We
will look into the details of each issue and verify the problem before
correcting it.
Kishore
On Tue, Feb 23, 2010 at 5:14 PM, Luis Ibanez <a class="moz-txt-link-rfc2396E" href="mailto:luis.ibanez@kitware.com"><luis.ibanez@kitware.com></a> wrote:
</pre>
<blockquote type="cite">
<pre wrap=""> Amy,
Thanks a lot for contributing these comments.
That's a great example of community building.
--
Arnaud, Kishore:
Do you think what we should make some of these
changes to the code in Insight/Code/Review ?
Or should we add comments to the documentation
of the classes ?
Thanks for any advice,
Luis
-----------------------------------------
On Tue, Feb 23, 2010 at 4:34 AM, Amy C <a class="moz-txt-link-rfc2396E" href="mailto:mathematical.coffee@gmail.com"><mathematical.coffee@gmail.com></a> wrote:
</pre>
<blockquote type="cite">
<pre wrap="">Hi,
For a while I've been using the Chan & Vese level set segmentation classes
from <a class="moz-txt-link-freetext" href="http://www.insight-journal.org/browse/publication/322">http://www.insight-journal.org/browse/publication/322</a> and
<a class="moz-txt-link-freetext" href="http://www.insight-journal.org/browse/publication/323">http://www.insight-journal.org/browse/publication/323</a>. (kudos to the
authors!)
For anyone planning to use this code in the future, here are some bugs I
noticed (I didn't really look at the sparse implementation, this is for the
function/dense filter code only):
- itkAtanRegularizedHeavisideStepFunction.h - in CalculateDerivative:
derivative should be 1/pi*1/epsilon*1/(1+(x/epsilon)^2), not
1/pi*1/(1+(x/epsilon)^2) (most people use epsilon==1 so there may be no
noticable difference)
- itkRegionBasedLevelSetFunction.txx - in ComputeGlobalTerm: in the
part that calculates the overlap term of the update, there's a call to
ComputeOverlapParameters which is only calculated when the overlap penalty
weight is non-zero. However ComputeOverlapParameters also calculates an
extra value called 'product' in the code, which holds product_{ i != current
function id } (1-Heaviside(level set i)). This term is required to calculate
the background term of the update (u_0 - backgroundAverage)^2*product. This
means that if the overlap penalty isn't 0 the background intensity term is
calculated correctly, but if the overlap penalty is 0, then '1' is used for
'product' instead of product_{ i != current function id } (1-Heaviside(level
set i)). Should be modified so that product is calculated regardless of the
overlap penalty.
- itkMultiphaseDenseFiniteDifferenceImageFilter - in ApplyUpdate: has
a variable m_ReinitializeCounter that is meant to reinitalise the level set
function to a signed distance function every m_ReinitalizeCounter
iterations. However at the moment if the filter doesn't reinitalize the
level set function, it won't update it either. (It should update every
iteration, reinitalize every m_ReinitializeCounter iterations instead of
only update & reinitalize every m_ReinitializeCounter iterations, and do
nothing on the other iterations). At the moment users won't notice a
difference because you can't set m_ReinitializeCounter with public access.
- itkMultiphaseDenseFiniteDifferenceImageFilter - in
CopyInputToOutput - at the moment calling GetOutput() on the dense level set
filter will return the segmentation given by the highest-index level set
function. It should really give the segmentation given by all level sets -
according to the sparse filter, GetOutput() should return an image that is
default 0, and takes the value i where the i'th level set indicates this
region is foreground (value < 0). In the case of overlap the highest index
wins. Just need to initialise the output with FillBuffer(0) and remove the
'else' clause from 'if (in.Get() < 0)'
- possibly want to make ComputeHessian virtual in
itkRegionBasedLevelSetFunction.h so that subclasses could override it
(debatable, not really a bug)
If unwilling to change the original files you could create a subclass and
override the respective functions with the fixes.
cheers,
Amy
_____________________________________
Powered by <a class="moz-txt-link-abbreviated" href="http://www.kitware.com">www.kitware.com</a>
Visit other Kitware open-source projects at
<a class="moz-txt-link-freetext" href="http://www.kitware.com/opensource/opensource.html">http://www.kitware.com/opensource/opensource.html</a>
Kitware offers ITK Training Courses, for more information visit:
<a class="moz-txt-link-freetext" href="http://www.kitware.com/products/protraining.html">http://www.kitware.com/products/protraining.html</a>
Please keep messages on-topic and check the ITK FAQ at:
<a class="moz-txt-link-freetext" href="http://www.itk.org/Wiki/ITK_FAQ">http://www.itk.org/Wiki/ITK_FAQ</a>
Follow this link to subscribe/unsubscribe:
<a class="moz-txt-link-freetext" href="http://www.itk.org/mailman/listinfo/insight-users">http://www.itk.org/mailman/listinfo/insight-users</a>
</pre>
</blockquote>
<pre wrap="">_____________________________________
Powered by <a class="moz-txt-link-abbreviated" href="http://www.kitware.com">www.kitware.com</a>
Visit other Kitware open-source projects at
<a class="moz-txt-link-freetext" href="http://www.kitware.com/opensource/opensource.html">http://www.kitware.com/opensource/opensource.html</a>
Kitware offers ITK Training Courses, for more information visit:
<a class="moz-txt-link-freetext" href="http://www.kitware.com/products/protraining.html">http://www.kitware.com/products/protraining.html</a>
Please keep messages on-topic and check the ITK FAQ at:
<a class="moz-txt-link-freetext" href="http://www.itk.org/Wiki/ITK_FAQ">http://www.itk.org/Wiki/ITK_FAQ</a>
Follow this link to subscribe/unsubscribe:
<a class="moz-txt-link-freetext" href="http://www.itk.org/mailman/listinfo/insight-users">http://www.itk.org/mailman/listinfo/insight-users</a>
</pre>
</blockquote>
<pre wrap="">_____________________________________
Powered by <a class="moz-txt-link-abbreviated" href="http://www.kitware.com">www.kitware.com</a>
Visit other Kitware open-source projects at
<a class="moz-txt-link-freetext" href="http://www.kitware.com/opensource/opensource.html">http://www.kitware.com/opensource/opensource.html</a>
Kitware offers ITK Training Courses, for more information visit:
<a class="moz-txt-link-freetext" href="http://www.kitware.com/products/protraining.html">http://www.kitware.com/products/protraining.html</a>
Please keep messages on-topic and check the ITK FAQ at:
<a class="moz-txt-link-freetext" href="http://www.itk.org/Wiki/ITK_FAQ">http://www.itk.org/Wiki/ITK_FAQ</a>
Follow this link to subscribe/unsubscribe:
<a class="moz-txt-link-freetext" href="http://www.itk.org/mailman/listinfo/insight-users">http://www.itk.org/mailman/listinfo/insight-users</a>
</pre>
</blockquote>
<br>
</body>
</html>