[Insight-developers] Rounding and itkMath.h (Class vs Namespace)
Bradley Lowekamp
blowekamp at mail.nih.gov
Thu Oct 8 10:22:30 EDT 2009
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.
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.
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20091008/9978852b/attachment.htm>
More information about the Insight-developers
mailing list