[Insight-developers] Rounding and itkMath.h (Class vs Namespace)
Tom Vercauteren
tom.vercauteren at m4x.org
Fri Oct 9 03:54:22 EDT 2009
Hi Brad,
On Thu, Oct 8, 2009 at 16:22, Bradley Lowekamp <blowekamp at mail.nih.gov> wrote:
>
> I have not devoted as much time to Math rounding method as I was hopping. I
> plan to do the first check in of this on Monday.
> One change I hope to make is to separate the implementation from the header
> file. As the Detail namespace declares and defines functions for inlining
> purposes, I don't think that making a "txx" file would be appropriate. I
> propose an itkMathDetail.h file for the "private" internal functions, which
> don't need to be in doxygen.
Sounds good. I like the idea of hiding the gory details, especially
with all the ifdef involved. If it still is too messy, another option
would be to have one "MathDetail" file per architecture:
* itkMathDetailVanilla.h
* itkMathDetailSSE2.h
* itkMathDetailGCCX86ASM.h
* itkMathDetailMSVCX86ASM.h
* etc.
In any case, it's nice not to show this in doxygen.
> Second, I know we designed the TReturn to expect integer types, but as it
> currently is it can be instantiated with real types as well. Also we must
> consider the behavior of unsigned integer return types as well. This gives
> us two options that I see, make it so it only compiles with integer types,
> or add support for real types.
Make it so it only compiles with integer types seems sufficient to me.
Bu if you feel we need to support real types, go ahead.
In our code we supported real types and unsigned types using boost mpl
(not sure hwo to do this using ITK only). The following code should be
sufficient to understand, just replace the iRound by you preferred
rounding function.
// Private function: rounding necessary
template<typename T, typename U>
inline U roundIfFloatingToIntegral_(T x, typename
boost::enable_if<boost::mpl::and_<
const boost::is_floating_point<T>,
boost::is_integral<U> > >::type * = 0)
{
return iRound(x);
}
// Private function: no rounding since we convert to floating point
template<typename T, typename U>
inline U roundIfFloatingToIntegral_(T x, typename
boost::disable_if<boost::mpl::and_<
const boost::is_floating_point<T>,
boost::is_integral<U> > >::type * = 0)
{
return static_cast<U> (x);
}
// convert T type to U type using round if U is integer (no special
care taken for unsigned)
template<typename T, typename U>
inline U roundIfFloatingToIntegral(T x)
{
return roundIfFloatingToIntegral_<T,U> (x);
}
// Private function: use smart T to U conversion and thresholding
above zero because U is unsigned
template<typename T, typename U>
inline U roundAndBoundIfFloatingToIntegral_(T x, typename
boost::enable_if<boost::mpl::and_<
boost::mpl::not_<boost::is_unsigned<T> >,
const
boost::is_unsigned<U> > >::type * = 0)
{
if(x > static_cast<T>(0)) return roundIfFloatingToIntegral<T,U>(x);
else return static_cast<U>(0);
}
// Private function: use smart T to U conversion, no need to threshold
above zero because U is signed
template<typename T, typename U>
inline U roundAndBoundIfFloatingToIntegral_(T x, typename
boost::disable_if<boost::mpl::and_<
boost::mpl::not_<boost::is_unsigned<T> >,
const
boost::is_unsigned<U> > >::type * = 0)
{
return roundIfFloatingToIntegral<T,U>(x);
}
// convert T type to U type using round if U is integer and
thresholding above zero if U is unsigned
template<typename T, typename U>
inline U roundAndBoundIfFloatingToIntegral(T x)
{
return roundAndBoundIfFloatingToIntegral_<T,U> (x);
}
> Third, we need more documentation. Does the
> itkTemplateFloatingToIntegerMacro work well with the Doxygen, so that
> Doxygen will make function documentation? I think is should since the
> SetGetMacros work. The documentation for the rounding methods should include
> a description expected usage, limitations and suggestions or references to
> where they should be used in ITK.
> Brad
> On Oct 1, 2009, at 10:37 AM, Tom Vercauteren wrote:
>
> Hey Brad,
>
> Moving forward on this would be very nice!
>
> I think the first step to solve before implementing improved template
> rounding functions is to get fixed-width (32 and 64 bit) integers. As
> I reported on the bug tracker
> http://www.itk.org/Bug/view.php?id=9426
> I believe that using pstdint.h
> http://www.azillionmonkeys.com/qed/pstdint.h
> should be the easiest portable way to go for that.
>
> Regarding the dummy argument. It's right that I have not used it in my
> itk-templatedmathround-2009-07-30.patch patch:
> http://www.itk.org/Bug/file_download.php?file_id=2390&type=bug
> The reason is that the dummy argument should only be necessary for
> MSVC 6 but since I have no access to this compiler, this is something
> I cannot be sure of. Trying first without the dummy argument and
> adding it only if the MSVC 6 dashboard build fails seems a reasonable
> plan.
>
> My two cents,
> Tom
>
> P.S.: I don't have much time to code on this right now but I'll try to
> be responsive to emails.
>
> On Thu, Oct 1, 2009 at 16:14, Bradley Lowekamp <blowekamp at mail.nih.gov>
> wrote:
>
> Hello,
>
> I would like to begin to determine the best way to implement the improved
>
> rounding methods across all the compilers we currently support. The plan is
>
> to create a itkMath.h file, and not include it int Macro.h until we have
>
> determined the best implementation. Then create a standalone test ( not part
>
> of a CTest driver ) to determine what is going to work best. This should
>
> only create one failing test/build until this is all figured out.
>
> Tom,
>
> The version in mantis is recent and doesn't include the dummy argument yet?
>
> The initial implementation we should try is without this dummy argument in a
>
> namespace.
>
> Brad
>
> On Aug 11, 2009, at 8:01 AM, Tom Vercauteren wrote:
>
> Hi Brad,
>
> From a totally subjective point of view, I prefer the namespace
>
> 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 = 3.14;
>
> namespace Detail
>
> {
>
> // Things here are excluded from the backward-compatibility policy
>
> template <typename TReturn, typename TInput>
>
> inline TReturn RoundHelper(TInput x, TReturn * = 0) { [snip] }
>
> }
>
> // The second argument is fro msvc 6 compatibility
>
> template <typename TReturn, typename TInput>
>
> inline TReturn Round(TInput x, TReturn * = 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 * = 0)
>
> {
>
> return RoundHelper<TReturn>(x);
>
> }
>
> private:
>
> template <typename TReturn, typename TInput>
>
> static inline TReturn RoundHelper(TInput x, TReturn * = 0) { [snip] }
>
> };
>
> const double Math::pi = 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 = 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 * = 0);
>
> private:
>
> template <typename TReturn, typename TInput>
>
> static inline TReturn RoundHelper(TInput x, TReturn * = 0) { [snip] }
>
> }
>
> template <typename TReturn, typename TInput>
>
> inline TReturn Round(TInput x, TReturn * = 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>
>
> wrote:
>
> 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 second
>
> is the numeric constants. For the numeric constants consider these two
>
> different possibilities:
>
>
> math.h:
>
> class Math
>
> {
>
> static const double p;
>
> }
>
> math.cxx:
>
> const double p = 3.14;
>
> Where as with a namespace, we don't need the static qualifier and I believe
>
> the following will not have the linking issues, and is valid c++:
>
> math.h:
>
> namespace Math {
>
> const double p = 3.14;
>
> };
>
>
> With the namespace implementation we can always get optimizations, but there
>
> will be multiple addresses in different compilation files. I don't believe
>
> that that latter is really a negative though.
>
>
> Brad
>
>
> ========================================================
>
> Bradley Lowekamp
>
> Lockheed Martin Contractor for
>
> Office of High Performance Computing and Communications
>
> National Library of Medicine
>
> blowekamp at mail.nih.gov
>
>
>
> ========================================================
>
> Bradley Lowekamp
>
> Lockheed Martin Contractor for
>
> Office of High Performance Computing and Communications
>
> National Library of Medicine
>
> blowekamp at mail.nih.gov
>
>
More information about the Insight-developers
mailing list