No subject
Mon Aug 10 18:29:21 EDT 2009
approach. The only issue I still have is about the backward<br>
compatibility policy. The rounding functions need some helper<br>
functions. It would be better if those helper functions were not<br>
subject to any backward compatibility requirement. Here are three<br>
possibilities I can think of:<br>
<br>
<br>
1) All namespace approach<br>
---------------------------------------<br>
Advantage: seems clearer to me<br>
Drawback: No control on the access to th helper functions in Math::Detail<b=
r>
<div class=3D"im"><br>
namespace Math<br>
=A0{<br>
=A0const double p =3D 3.14;<br>
<br>
</div> =A0namespace Detail<br>
=A0 =A0{<br>
=A0 =A0// Things here are excluded from the backward-compatibility policy<=
br>
=A0 =A0template <typename TReturn, typename TInput><br>
=A0 =A0inline TReturn RoundHelper(TInput x, TReturn * =3D 0) { [snip] }<br=
>
=A0 =A0}<br>
<br>
=A0// The second argument is fro msvc 6 compatibility<br>
=A0template <typename TReturn, typename TInput><br>
=A0inline TReturn Round(TInput x, TReturn * =3D 0)<br>
=A0 =A0{<br>
=A0 =A0return Detail::RoundHelper<TReturn>(x);<br>
=A0 =A0}<br>
=A0}<br>
<br>
<br>
<br>
2) All class approach<br>
--------------------------------<br>
Advantage: Helper functions not accessible to the users<br>
Drawback: Somehow confusing for me to have a class with only static<br>
members and functions (Also potential compiler optimization loss??)<br>
<br>
class Math<br>
{<br>
public:<br>
=A0static const double pi;<br>
<br>
=A0template <typename TReturn, typename TInput><br>
=A0static inline TReturn Round(TInput x, TReturn * =3D 0)<br>
=A0 =A0{<br>
=A0 =A0return RoundHelper<TReturn>(x);<br>
=A0 =A0}<br>
<br>
private:<br>
=A0template <typename TReturn, typename TInput><br>
=A0static inline TReturn RoundHelper(TInput x, TReturn * =3D 0) { [snip] }=
<br>
};<br>
<br>
const double Math::pi =3D 3.14;<br>
<br>
<br>
<br>
3) Mixed namespace-class approach<br>
-----------------------------------------------------<br>
Advantage: Helper functions not accessible to the users, constants are<br>
defined next to their declaration<br>
Drawback: More complex code due to template friend functions<br>
<div class=3D"im"><br>
namespace Math<br>
=A0{<br>
=A0const double p =3D 3.14;<br>
<br>
</div> =A0// Forward declarations<br>
=A0class MathDetail;<br>
=A0template<typename TReturn, typename TInput><br>
=A0TReturn =A0Round(TInput, TReturn *);<br>
<br>
=A0class MathDetail<br>
=A0{<br>
=A0public:<br>
=A0 =A0template<typename TReturn, typename TInput><br>
=A0 =A0friend inline TReturn Round(TInput x, TReturn * =3D 0);<br>
<br>
=A0private:<br>
=A0 =A0template <typename TReturn, typename TInput><br>
=A0 =A0static inline TReturn RoundHelper(TInput x, TReturn * =3D 0) { [sni=
p] }<br>
=A0}<br>
<br>
=A0template <typename TReturn, typename TInput><br>
=A0inline TReturn Round(TInput x, TReturn * =3D 0)<br>
=A0 =A0{<br>
=A0 =A0return MathDetail::RoundHelper<TReturn>(x);<br>
=A0 =A0}<br>
=A0}<br>
<br>
<br>
It seems to me we should wait until after the release to implement any<br>
of these option. What do you think?<br>
<br>
Cheers,<br>
<font color=3D"#888888">Tom<br>
</font><div><div></div><div class=3D"h5"><br>
<br>
On Sat, Aug 8, 2009 at 16:18, Bradley Lowekamp<<a href=3D"mailto:bloweka=
mp at mail.nih.gov">blowekamp at mail.nih.gov</a>> wrote:<br>
> Currently we have two things we would like to put into itk::Math, weat=
her it<br>
> be a namespace or a class. The first is the rounding methods and the s=
econd<br>
> is the numeric constants. For the numeric constants consider these two=
<br>
> different possibilities:<br>
><br>
><br>
> math.h:<br>
> class Math<br>
> {<br>
> =A0 =A0 =A0 =A0static const double p;<br>
> }<br>
><br>
> math.cxx:<br>
> const double p =3D 3.14;<br>
><br>
> Where as with a namespace, we don't need the static qualifier and =
I believe<br>
> the following will not have the linking issues, and is valid c++:<br>
><br>
> math.h:<br>
> namespace Math {<br>
> =A0 =A0 =A0 =A0const double p =3D 3.14;<br>
> };<br>
><br>
><br>
> With the namespace implementation we can always get optimizations, but=
there<br>
> will be multiple addresses in different compilation files. I don't=
believe<br>
> that that latter is really a negative though.<br>
><br>
><br>
> Brad<br>
><br>
><br>
> On Aug 6, 2009, at 11:21 AM, Tom Vercauteren wrote:<br>
><br>
>> Hi Brad,<br>
>><br>
>> Moving these mathematical constants to a more visible place sounds=
<br>
>> good to me. However, handling floating point constants in c++ is a=
<br>
>> pain. For example, the following code is not standard and won'=
t be<br>
>> usable on several systems (mac gcc 4.0 at least):<br>
>><br>
>> class No_Good {<br>
>> =A0static double const d =3D 1.0;<br>
>> };<br>
>><br>
>> I guess that's why boost chose to implement these constants as=
<br>
>> template functions:<br>
>><br>
>> <a href=3D"http://www.boost.org/doc/libs/1_39_0/libs/math/doc/sf_a=
nd_dist/html/math_toolkit/toolkit/internals1/constants.html" target=3D"_bla=
nk">http://www.boost.org/doc/libs/1_39_0/libs/math/doc/sf_and_dist/html/mat=
h_toolkit/toolkit/internals1/constants.html</a><br>
>><br>
>> The other option is to decouple the definition and declaration as =
done<br>
>> in vnl =A0(with some ugly macro):<br>
>><br>
>> class Now_Better<br>
>> {<br>
>> =A0 static double const d;<br>
>> };<br>
>><br>
>> double const Now_Better::d =3D 1.0;<br>
>><br>
>> More information can be found here:<br>
>><br>
>> <a href=3D"http://stackoverflow.com/questions/370283/why-cant-i-ha=
ve-a-non-integral-static-const-member-in-a-class" target=3D"_blank">http://=
stackoverflow.com/questions/370283/why-cant-i-have-a-non-integral-static-co=
nst-member-in-a-class</a><br>
>><br>
>> No matter the implementation, I would definitely vote for replacin=
g<br>
>> the current hard-coded values by a common thing such as vnl_math::=
pi,<br>
>> itk::Math::pi, or =A0itk::Math::pi<double>().<br>
>><br>
>> My two cents,<br>
>> Tom<br>
>><br>
>> On Thu, Aug 6, 2009 at 16:40, Bradley Lowekamp<<a href=3D"mailt=
o:blowekamp at mail.nih.gov">blowekamp at mail.nih.gov</a>><br>
>> wrote:<br>
>>><br>
>>> Hello,<br>
>>><br>
>>> =A0 =A0 =A0 I was looking at:<br>
>>><br>
>>> BasicFilters/itkBilateralImageFilter.txx: =A0rangeGaussianDeno=
m =3D<br>
>>> m_RangeSigma*vcl_sqrt(2.0*3.1415927);<br>
>>><br>
>>> It took me a little bit to figure out that we most likely shou=
ld be using<br>
>>> vnl_math::pi, along with their "e and all that". I d=
o wonder if these<br>
>>> constants were in itk::Math and in doxygen if it would help wi=
th<br>
>>> increased<br>
>>> usage.<br>
>>><br>
>>> So I began greping in the "Code" directory and came =
up with the<br>
>>> following:<br>
>>><br>
>>> grep -r 3.1415 *<br>
>>> BasicFilters/itkBilateralImageFilter.txx: =A0rangeGaussianDeno=
m =3D<br>
>>> m_RangeSigma*vcl_sqrt(2.0*3.1415927);<br>
>>> BasicFilters/itkVectorGradientMagnitudeImageFilter.txx: =A0con=
st double dpi<br>
>>> =3D<br>
>>> 3.14159265358979323846;<br>
>>> Common/itkGaussianDerivativeSpatialFunction.txx: =A0 =A0prefix=
Denom *=3D<br>
>>> 2*vcl_pow( 2 * 3.1415927, VImageDimension / 2.0);<br>
>>> Common/itkMersenneTwisterRandomVariateGenerator.h: =A0double p=
hi =3D 2.0 *<br>
>>> 3.14159265358979323846264338328<br>
>>> Numerics/itkCumulativeGaussianOptimizer.cxx: =A0m_ComputedAmpl=
itude =3D =A0sum<br>
>>> /<br>
>>> (m_ComputedStandardDeviation * vcl_sqrt(2*3.14159265));<br>
>>><br>
>>> grep -r 0.707 *<br>
>>> BasicFilters/itkBSplineResampleImageFilterBase.txx: =A0 =A0 =
=A0m_G[0] =A0=3D<br>
>>> =A00.707107;<br>
>>><br>
>>> grep -r 1.414 *<br>
>>> Numerics/Statistics/itkGaussianDistribution.cxx: =A0 =A0dq =A0=
=3D 0.5e+0 *<br>
>>> vnl_erfc(<br>
>>> dx / 1.414213562373095e+0 ) - dp;<br>
>>> Review/Statistics/itkGaussianDistribution.cxx: =A0 =A0dq =A0=
=3D 0.5e+0 *<br>
>>> vnl_erfc(<br>
>>> dx / 1.414213562373095e+0 ) - dp;<br>
>>><br>
>>> grep -r vnl_math:: * | wc<br>
>>> =A0 =A0 69 =A0 =A0 571 =A0 =A07652<br>
>>><br>
>>> Well I guess that is almost 90% usage of the numeric constants=
, so its<br>
>>> not<br>
>>> bad at all.<br>
>>><br>
>>> I'll work on a patch tonight, and likely commit something =
tomorrow if no<br>
>>> one<br>
>>> sees a problem with this.<br>
>>><br>
>>> Brad<br>
>>> _______________________________________________<br>
>>> Powered by <a href=3D"http://www.kitware.com" target=3D"_blank=
">www.kitware.com</a><br>
>>><br>
>>> Visit other Kitware open-source projects at<br>
>>> <a href=3D"http://www.kitware.com/opensource/opensource.html" =
target=3D"_blank">http://www.kitware.com/opensource/opensource.html</a><br>
>>><br>
>>> Please keep messages on-topic and check the ITK FAQ at:<br>
>>> <a href=3D"http://www.itk.org/Wiki/ITK_FAQ" target=3D"_blank">=
http://www.itk.org/Wiki/ITK_FAQ</a><br>
>>><br>
>>> Follow this link to subscribe/unsubscribe:<br>
>>> <a href=3D"http://www.itk.org/mailman/listinfo/insight-develop=
ers" target=3D"_blank">http://www.itk.org/mailman/listinfo/insight-develope=
rs</a><br>
>>><br>
><br>
><br>
_______________________________________________<br>
Powered by <a href=3D"http://www.kitware.com" target=3D"_blank">www.kitware=
.com</a><br>
<br>
Visit other Kitware open-source projects at <a href=3D"http://www.kitware.c=
om/opensource/opensource.html" target=3D"_blank">http://www.kitware.com/ope=
nsource/opensource.html</a><br>
<br>
Please keep messages on-topic and check the ITK FAQ at: <a href=3D"http://w=
ww.itk.org/Wiki/ITK_FAQ" target=3D"_blank">http://www.itk.org/Wiki/ITK_FAQ<=
/a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href=3D"http://www.itk.org/mailman/listinfo/insight-developers" target=
=3D"_blank">http://www.itk.org/mailman/listinfo/insight-developers</a><br>
</div></div></blockquote></div><br>
--0015175ce1427ee5460470de04c3--
More information about the Insight-developers
mailing list