No subject
Mon Aug 10 18:29:21 EDT 2009
approach. The only issue I still have is about the backward
compatibility policy. The rounding functions need some helper
functions. It would be better if those helper functions were not
subject to any backward compatibility requirement. Here are three
possibilities I can think of:
1) All namespace approach
---------------------------------------
Advantage: seems clearer to me
Drawback: No control on the access to th helper functions in Math::Detail
namespace Math
{
const double p =3D 3.14;
namespace Detail
{
// Things here are excluded from the backward-compatibility policy
template <typename TReturn, typename TInput>
inline TReturn RoundHelper(TInput x, TReturn * =3D 0) { [snip] }
}
// The second argument is fro msvc 6 compatibility
template <typename TReturn, typename TInput>
inline TReturn Round(TInput x, TReturn * =3D 0)
{
return Detail::RoundHelper<TReturn>(x);
}
}
2) All class approach
--------------------------------
Advantage: Helper functions not accessible to the users
Drawback: Somehow confusing for me to have a class with only static
members and functions (Also potential compiler optimization loss??)
class Math
{
public:
static const double pi;
template <typename TReturn, typename TInput>
static inline TReturn Round(TInput x, TReturn * =3D 0)
{
return RoundHelper<TReturn>(x);
}
private:
template <typename TReturn, typename TInput>
static inline TReturn RoundHelper(TInput x, TReturn * =3D 0) { [snip] }
};
const double Math::pi =3D 3.14;
3) Mixed namespace-class approach
-----------------------------------------------------
Advantage: Helper functions not accessible to the users, constants are
defined next to their declaration
Drawback: More complex code due to template friend functions
namespace Math
{
const double p =3D 3.14;
// Forward declarations
class MathDetail;
template<typename TReturn, typename TInput>
TReturn Round(TInput, TReturn *);
class MathDetail
{
public:
template<typename TReturn, typename TInput>
friend inline TReturn Round(TInput x, TReturn * =3D 0);
private:
template <typename TReturn, typename TInput>
static inline TReturn RoundHelper(TInput x, TReturn * =3D 0) { [snip] }
}
template <typename TReturn, typename TInput>
inline TReturn Round(TInput x, TReturn * =3D 0)
{
return MathDetail::RoundHelper<TReturn>(x);
}
}
It seems to me we should wait until after the release to implement any
of these option. What do you think?
Cheers,
Tom
On Sat, Aug 8, 2009 at 16:18, Bradley Lowekamp<blowekamp at mail.nih.gov> wrot=
e:
> Currently we have two things we would like to put into itk::Math, weather=
it
> be a namespace or a class. The first is the rounding methods and the seco=
nd
> is the numeric constants. For the numeric constants consider these two
> different possibilities:
>
>
> math.h:
> class Math
> {
> =A0 =A0 =A0 =A0static const double p;
> }
>
> math.cxx:
> const double p =3D 3.14;
>
> Where as with a namespace, we don't need the static qualifier and I belie=
ve
> the following will not have the linking issues, and is valid c++:
>
> math.h:
> namespace Math {
> =A0 =A0 =A0 =A0const double p =3D 3.14;
> };
>
>
> With the namespace implementation we can always get optimizations, but th=
ere
> will be multiple addresses in different compilation files. I don't believ=
e
> that that latter is really a negative though.
>
>
> Brad
>
>
> On Aug 6, 2009, at 11:21 AM, Tom Vercauteren wrote:
>
>> Hi Brad,
>>
>> Moving these mathematical constants to a more visible place sounds
>> good to me. However, handling floating point constants in c++ is a
>> pain. For example, the following code is not standard and won't be
>> usable on several systems (mac gcc 4.0 at least):
>>
>> class No_Good {
>> =A0static double const d =3D 1.0;
>> };
>>
>> I guess that's why boost chose to implement these constants as
>> template functions:
>>
>> http://www.boost.org/doc/libs/1_39_0/libs/math/doc/sf_and_dist/html/math=
_toolkit/toolkit/internals1/constants.html
>>
>> The other option is to decouple the definition and declaration as done
>> in vnl =A0(with some ugly macro):
>>
>> class Now_Better
>> {
>> =A0 static double const d;
>> };
>>
>> double const Now_Better::d =3D 1.0;
>>
>> More information can be found here:
>>
>> http://stackoverflow.com/questions/370283/why-cant-i-have-a-non-integral=
-static-const-member-in-a-class
>>
>> No matter the implementation, I would definitely vote for replacing
>> the current hard-coded values by a common thing such as vnl_math::pi,
>> itk::Math::pi, or =A0itk::Math::pi<double>().
>>
>> My two cents,
>> Tom
>>
>> On Thu, Aug 6, 2009 at 16:40, Bradley Lowekamp<blowekamp at mail.nih.gov>
>> wrote:
>>>
>>> Hello,
>>>
>>> =A0 =A0 =A0 I was looking at:
>>>
>>> BasicFilters/itkBilateralImageFilter.txx: =A0rangeGaussianDenom =3D
>>> m_RangeSigma*vcl_sqrt(2.0*3.1415927);
>>>
>>> It took me a little bit to figure out that we most likely should be usi=
ng
>>> vnl_math::pi, along with their "e and all that". I do wonder if these
>>> constants were in itk::Math and in doxygen if it would help with
>>> increased
>>> usage.
>>>
>>> So I began greping in the "Code" directory and came up with the
>>> following:
>>>
>>> grep -r 3.1415 *
>>> BasicFilters/itkBilateralImageFilter.txx: =A0rangeGaussianDenom =3D
>>> m_RangeSigma*vcl_sqrt(2.0*3.1415927);
>>> BasicFilters/itkVectorGradientMagnitudeImageFilter.txx: =A0const double=
dpi
>>> =3D
>>> 3.14159265358979323846;
>>> Common/itkGaussianDerivativeSpatialFunction.txx: =A0 =A0prefixDenom *=
=3D
>>> 2*vcl_pow( 2 * 3.1415927, VImageDimension / 2.0);
>>> Common/itkMersenneTwisterRandomVariateGenerator.h: =A0double phi =3D 2.=
0 *
>>> 3.14159265358979323846264338328
>>> Numerics/itkCumulativeGaussianOptimizer.cxx: =A0m_ComputedAmplitude =3D=
=A0sum
>>> /
>>> (m_ComputedStandardDeviation * vcl_sqrt(2*3.14159265));
>>>
>>> grep -r 0.707 *
>>> BasicFilters/itkBSplineResampleImageFilterBase.txx: =A0 =A0 =A0m_G[0] =
=A0=3D
>>> =A00.707107;
>>>
>>> grep -r 1.414 *
>>> Numerics/Statistics/itkGaussianDistribution.cxx: =A0 =A0dq =A0=3D 0.5e+=
0 *
>>> vnl_erfc(
>>> dx / 1.414213562373095e+0 ) - dp;
>>> Review/Statistics/itkGaussianDistribution.cxx: =A0 =A0dq =A0=3D 0.5e+0 =
*
>>> vnl_erfc(
>>> dx / 1.414213562373095e+0 ) - dp;
>>>
>>> grep -r vnl_math:: * | wc
>>> =A0 =A0 69 =A0 =A0 571 =A0 =A07652
>>>
>>> Well I guess that is almost 90% usage of the numeric constants, so its
>>> not
>>> bad at all.
>>>
>>> I'll work on a patch tonight, and likely commit something tomorrow if n=
o
>>> one
>>> sees a problem with this.
>>>
>>> Brad
>>> _______________________________________________
>>> Powered by 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
>>>
>
>
More information about the Insight-developers
mailing list