Minor refinements for boost::rational
Hello, After a short view at the rational.hpp header I propose to perform the following changes: 1) As counterpart to the existing: bool operator!() const; one should definitly add: operator bool_replacement_type() const; with a proper (member function pointer) definition of bool_replacement_type. 2) The function template template <typename IntType> IntType lcm(IntType n, IntType m); should be defined as template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m); which makes sense in this case because m is not modified internally (in contrast to n) 3) The boost::rational IO should use proper templated extractors and inserters for those (most) compilers, which provide corresponding std IO libraries, such that #if defined (BOOST_NO_TEMPLATED_STREAMS) ... // current impl #else template<class CharType, class CharTrait, class IntType> inline std::basic_istream<CharType, CharTrait>& operator>>(std::basic_istream<CharType, CharTrait> &is, rational<IntType>&); template<class CharType, class CharTrait, class IntType> inline std::basic_ostream<CharType, CharTrait>& operator<<(std::basic_ostream<CharType, CharTrait> &os, const rational<IntType>&); #end Furtheron I propose to replace the helper class detail::resetter by boost::ios_flags_saver, or even better to remove it all all, because I assume, that the temporary unsetting of the skipws flag does not make much sense - Does it? I also propose to replace the current setting of ios_base::badbit (if widen('/') was not found) by the more appropriate setting of ios_base::failbit - This behaviour is in sync with std::complex and boost::tuple! Greetings from Bremen, Daniel Krügler
On 8/22/05 2:34 AM, "Daniel Krügler" <dsp@bdal.de> wrote:
After a short view at the rational.hpp header I propose to perform the following changes:
1) As counterpart to the existing:
bool operator!() const;
one should definitly add:
operator bool_replacement_type() const;
with a proper (member function pointer) definition of bool_replacement_type.
I just sent a post suggesting that on the developer list. (I already filed a bug about it a few weeks ago.)
2) The function template
template <typename IntType> IntType lcm(IntType n, IntType m);
should be defined as
template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m);
which makes sense in this case because m is not modified internally (in contrast to n) [TRUNCATE refined rational I/O ideas]
Wouldn't that be giving away an implementation detail? I wonder if we could use the gcd & lcm functions I defined in boost::math? (They were written after boost::rational was.) -- Daryle Walker Mac, Internet, and Video Game Junkie darylew AT hotmail DOT com
Daryle Walker wrote:
Wouldn't that be giving away an implementation detail? I wonder if we could use the gcd & lcm functions I defined in boost::math? (They were written after boost::rational was.)
1) The argumentation should be vice versa: The **current** interface of the gcd and lcm function templates **is** implementation-driven! The reason is that these functions would otherwise need to make a copy of their arguments - all but not the second argument of the lcm function. This is also documented in the code of gcd function template. This might sound nitty-picky on the first view, but please consider that the rational class template is explicitely designed to handle arbitrary integer-like underlying types, thus including a possible unlimited-precision integer type (which is currently under proposal for standard C++) - this is explicitely expressed in http://www.boost.org/libs/rational/rational.html Therefore any **unnecessary** argument copies should be prevented. I proposed boost::call_traits because it was already used in this header (in the definition of the rational class template itself). 2) It seems as if the functions of common_factor_rt are prepared for that, although I only skimmed over the code. Honestly from the current boost docs in http://www.boost.org/libs/math/doc/common_factor.html I could not extract the exact requirements on the template parameter for common_factor_rt. It just says "This type should be a numeric type that represents integers". What does that mean under the language restrictions of C++? (Note that the boost::rational docs describe specific requirements on this type, see corresponding section "Integer Type Requirements") Greetings, Daniel
On 8/23/05 2:17 AM, "Daniel Krügler" <dsp@bdal.de> wrote:
2) It seems as if the functions of common_factor_rt are prepared for that, although I only skimmed over the code. Honestly from the current boost docs in
(I wrote this.)
I could not extract the exact requirements on the template parameter for common_factor_rt. It just says "This type should be a numeric type that represents integers". What does that mean under the language restrictions of C++?
The type's operations should be like the ones for built-in integers. For instance, the functions need "quotient & remainder" type division.
(Note that the boost::rational docs describe specific requirements on this type, see corresponding section "Integer Type Requirements")
Basically any type that matches the description for boost::rational should also work for the common-factor functions (since my code was inspired by Paul's). -- Daryle Walker Mac, Internet, and Video Game Junkie darylew AT hotmail DOT com
On 8/22/05 2:34 AM, "Daniel Krügler" <dsp@bdal.de> wrote: [SNIP]
2) The function template
template <typename IntType> IntType lcm(IntType n, IntType m);
should be defined as
template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m);
which makes sense in this case because m is not modified internally (in contrast to n) [TRUNCATE]
I saw a note from someone who is proposing an alternate to the boost::any class: * Should call_traits be used for templated assignment/construction? [No, some searching in the boost mailing list suggested that it was an obsolete optimization, and likely didn't make a difference anymore] but I don't know how accurate it is, or if it's applicable to non-member function templates. -- Daryle Walker Mac, Internet, and Video Game Junkie darylew AT hotmail DOT com
Daniel Krügler wrote:
Hello,
After a short view at the rational.hpp header I propose to perform the following changes:
There will be at least one major change in boost.Rational soon, at which time I'll probably make a few small changes, including addint a safe bool conversion and cleaning up i/o.
1) As counterpart to the existing:
bool operator!() const;
one should definitly add:
operator bool_replacement_type() const;
with a proper (member function pointer) definition of bool_replacement_type.
2) The function template
template <typename IntType> IntType lcm(IntType n, IntType m);
should be defined as
template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m);
which makes sense in this case because m is not modified internally (in contrast to n)
I think lcm should be symmetrical.
3) The boost::rational IO should use proper templated extractors and inserters for those (most) compilers, which provide corresponding std IO libraries, such that
#if defined (BOOST_NO_TEMPLATED_STREAMS) ... // current impl #else
template<class CharType, class CharTrait, class IntType> inline std::basic_istream<CharType, CharTrait>& operator>>(std::basic_istream<CharType, CharTrait> &is, rational<IntType>&);
template<class CharType, class CharTrait, class IntType> inline std::basic_ostream<CharType, CharTrait>& operator<<(std::basic_ostream<CharType, CharTrait> &os, const rational<IntType>&);
#end
Furtheron I propose to replace the helper class detail::resetter by boost::ios_flags_saver, or even better to remove it all all, because I assume, that the temporary unsetting of the skipws flag does not make much sense - Does it?
I also propose to replace the current setting of ios_base::badbit (if widen('/') was not found) by the more appropriate setting of ios_base::failbit - This behaviour is in sync with std::complex and boost::tuple!
Sounds fine.
Greetings from Bremen,
Daniel Krügler
Greeting from ... Utah :( Jonathan
Jonathan Turkanis wrote:
Daniel Krügler wrote:
Hello,
After a short view at the rational.hpp header I propose to perform the following changes:
There will be at least one major change in boost.Rational soon, at which time I'll probably make a few small changes, including addint a safe bool conversion and cleaning up i/o.
That sounds fine.
2) The function template
template <typename IntType> IntType lcm(IntType n, IntType m);
should be defined as
template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m);
which makes sense in this case because m is not modified internally (in contrast to n)
I think lcm should be symmetrical.
1) Wow, not so much of details of reasonings, please ;-)) 2) OK, the request of symmetry can lead to several conclusions: If we argue that the interface should not reflect the implementation, the most reasonable signature would be something like template <typename IntType> IntType lcm(const IntTyp& n, const IntTyp& m); (We cannot use the optimal version template <typename IntType> IntType lcm(boost::call_traits<IntType>::param_type n, boost::call_traits<IntType>::param_type m); because that would prevent argument deduction) Otherway around we could argue, that we want to use advantages of a special copy-optimization technique. This is an implementation-driven interface, but there are cases, where proposers (I am not one) have argued that the advantages outweight its disadvantages. If I try to compare these to approaches not too strongly subjectively (its impossible, I know, but I'll try ;-))), than I would say, that an implementation-driven interface should be only used where necessary, but not otherwise. This lead to my proposal to use call-by-value only for the first, not for the second argument of lcm. I don't understand, **why** you think, that the interface must/should be symmetric: 1) Thinking not implementation-driven, than template <typename IntType> IntType lcm(const IntTyp& n, const IntTyp& m); would be right thing for general IntTypes (see std::max/min interface). I would agree to that proposal 2) Thinking implementation-driven, than template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m); must be acceptable and **even more reasonable** than the currently existing template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m); interface. Greetings, Daniel
Daniel Krügler wrote:
Jonathan Turkanis wrote:
Daniel Krügler wrote:
2) The function template
template <typename IntType> IntType lcm(IntType n, IntType m);
should be defined as
template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m);
which makes sense in this case because m is not modified internally (in contrast to n)
I think lcm should be symmetrical.
1) Wow, not so much of details of reasonings, please ;-))
Because the semantics are symmetrical. The above asymmetry wouldn't be acceptable for max or +, would it? How much performance improvement do you expect, and in what cases?
2) OK, the request of symmetry can lead to several conclusions:
If we argue that the interface should not reflect the implementation, the most reasonable signature would be something like
template <typename IntType> IntType lcm(const IntTyp& n, const IntTyp& m);
(We cannot use the optimal version
template <typename IntType> IntType lcm(boost::call_traits<IntType>::param_type n, boost::call_traits<IntType>::param_type m);
because that would prevent argument deduction)
Otherway around we could argue, that we want to use advantages of a special copy-optimization technique. This is an implementation-driven interface, but there are cases, where proposers (I am not one) have argued that the advantages outweight its disadvantages.
Okay, what are you arguing, then?
If I try to compare these to approaches not too strongly subjectively (its impossible, I know, but I'll try ;-))), than I would say, that an implementation-driven interface should be only used where necessary, but not otherwise. This lead to my proposal to use call-by-value only for the first, not for the second argument of lcm.
I don't understand, **why** you think, that the interface must/should be symmetric:
1) Thinking not implementation-driven, than
template <typename IntType> IntType lcm(const IntTyp& n, const IntTyp& m);
would be right thing for general IntTypes (see std::max/min interface). I would agree to that proposal
Are you saying this is always better than pass by value, or just sometimes?
2) Thinking implementation-driven, than
template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m);
must be acceptable and **even more reasonable** than the currently existing
template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m);
interface.
They look the same to me.
Greetings,
Daniel
Jonathan
Jonathan Turkanis wrote:
Daniel Krügler wrote:
Jonathan Turkanis wrote:
Daniel Krügler wrote:
2) The function template
template <typename IntType> IntType lcm(IntType n, IntType m);
should be defined as
template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m);
which makes sense in this case because m is not modified internally (in contrast to n)
I think lcm should be symmetrical.
1) Wow, not so much of details of reasonings, please ;-))
Because the semantics are symmetrical. The above asymmetry wouldn't be acceptable for max or +, would it?
No, because they would use template <typename IntType> IntType lcm(const IntTyp& n, const IntTyp& m); as shown in my previous posting.
How much performance improvement do you expect, and in what cases?
I expect performance differences in case of (most probably dynamically growing) arbitrary-precision integer types, as described in my original posting. As explained in detail by referencing the boost docs it is an explicit intention of the current design. Following my reasoning we currently have one unnecessary copy of one the 2nd argument in lcm and both of my proposals do not have this deficiency. I think we don't have to discuss about cases where the arguments are native integral types, but we have to consider something like std::integer, see http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2005/n1744.pdf
2) OK, the request of symmetry can lead to several conclusions:
If we argue that the interface should not reflect the implementation, the most reasonable signature would be something like
template <typename IntType> IntType lcm(const IntTyp& n, const IntTyp& m);
(We cannot use the optimal version
template <typename IntType> IntType lcm(boost::call_traits<IntType>::param_type n, boost::call_traits<IntType>::param_type m);
because that would prevent argument deduction)
Otherway around we could argue, that we want to use advantages of a special copy-optimization technique. This is an implementation-driven interface, but there are cases, where proposers (I am not one) have argued that the advantages outweight its disadvantages.
Okay, what are you arguing, then?
Interuption of the sentences does not make sense here. I think the answer follows from the complete paragraph and I do answer below.
If I try to compare these to approaches not too strongly subjectively (its impossible, I know, but I'll try ;-))), than I would say, that an implementation-driven interface should be only used where necessary, but not otherwise. This lead to my proposal to use call-by-value only for the first, not for the second argument of lcm.
I don't understand, **why** you think, that the interface must/should be symmetric:
1) Thinking not implementation-driven, than
template <typename IntType> IntType lcm(const IntTyp& n, const IntTyp& m);
would be right thing for general IntTypes (see std::max/min interface). I would agree to that proposal
Are you saying this is always better than pass by value, or just sometimes?
As **one** general solution this would be best, yes. Currently there is only one general solution, so I am discussing about that. Call by value would be probably somewhat better for native integral types (This is the reason for call_traits). This fact was the reason for my original proposal of the asymmetric version. Thinking in **one** general solution, I propose either the asymmetric version or the version using reference arguments as both arguments, whatever you like. I only wanted to show a direction which to my opinion is better than the current state, I don't say that my proposal is the best of the set of all possible solutions. If one prefers finer granulation one could of course replace the single general solution by SFINAE-overloading techniques to handle cheap-copyable types by others and if any one would prefer that, I would applaud to that.
2) Thinking implementation-driven, than
template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m);
must be acceptable and **even more reasonable** than the currently existing
template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m);
interface.
They look the same to me.
Sorry, I made a mistake here. My (asymmetric) proposal is: template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m); and the current state is: template <typename IntType> IntType lcm(IntType n, IntType m); Daniel
Daniel Krügler wrote:
Jonathan Turkanis wrote:
Daniel Krügler wrote:
Jonathan Turkanis wrote:
Daniel Krügler wrote:
2) The function template
template <typename IntType> IntType lcm(IntType n, IntType m);
should be defined as
template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m);
which makes sense in this case because m is not modified internally (in contrast to n)
I think lcm should be symmetrical.
1) Wow, not so much of details of reasonings, please ;-))
Because the semantics are symmetrical. The above asymmetry wouldn't be acceptable for max or +, would it?
No, because they would use
template <typename IntType> IntType lcm(const IntTyp& n, const IntTyp& m);
as shown in my previous posting.
My point was: even if the assymetrical version were more efficient, the interface would be unacceptable.
How much performance improvement do you expect, and in what cases?
I expect performance differences in case of (most probably dynamically growing) arbitrary-precision integer types, as described in my original posting. As explained in detail by referencing the boost docs it is an explicit intention of the current design.
I guess I missed your reference to the docs.
Following my reasoning we currently have one unnecessary copy of one the 2nd argument in lcm and both of my proposals do not have this deficiency. I think we don't have to discuss about cases where the arguments are native integral types, but we have to consider something like std::integer, see
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2005/n1744.pdf
Okay. Pass by value should be okay for built-in integral types, then.
2) OK, the request of symmetry can lead to several conclusions:
If we argue that the interface should not reflect the implementation, the most reasonable signature would be something like
template <typename IntType> IntType lcm(const IntTyp& n, const IntTyp& m);
(We cannot use the optimal version
template <typename IntType> IntType lcm(boost::call_traits<IntType>::param_type n, boost::call_traits<IntType>::param_type m);
because that would prevent argument deduction)
Otherway around we could argue, that we want to use advantages of a special copy-optimization technique. This is an implementation-driven interface, but there are cases, where proposers (I am not one) have argued that the advantages outweight its disadvantages.
Okay, what are you arguing, then?
Interuption of the sentences does not make sense here. I think the answer follows from the complete paragraph and I do answer below.
I put my comment between two of your paragraphs. You're the one who interrupted the sentences ;-)
If I try to compare these to approaches not too strongly subjectively (its impossible, I know, but I'll try ;-))), than I would say, that an implementation-driven interface should be only used where necessary, but not otherwise. This lead to my proposal to use call-by-value only for the first, not for the second argument of lcm.
I don't understand, **why** you think, that the interface must/should be symmetric:
1) Thinking not implementation-driven, than
template <typename IntType> IntType lcm(const IntTyp& n, const IntTyp& m);
would be right thing for general IntTypes (see std::max/min interface). I would agree to that proposal
Are you saying this is always better than pass by value, or just sometimes?
As **one** general solution this would be best, yes. Currently there is only one general solution, so I am discussing about that.
Call by value would be probably somewhat better for native integral types (This is the reason for call_traits). This fact was the reason for my original proposal of the asymmetric version.
Thinking in **one** general solution, I propose either the asymmetric version or the version using reference arguments as both arguments, whatever you like. I only wanted to show a direction which to my opinion is better than the current state, I don't say that my proposal is the best of the set of all possible solutions.
If one prefers finer granulation one could of course replace the single general solution by SFINAE-overloading techniques to handle cheap-copyable types by others and if any one would prefer that, I would applaud to that.
Why do we need SFINAE? I can just define overloads for built-in intergal types, and a templated version that uses const references.
2) Thinking implementation-driven, than
template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m);
must be acceptable and **even more reasonable** than the currently existing
template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m);
interface.
They look the same to me.
Sorry, I made a mistake here. My (asymmetric) proposal is:
template <typename IntType> IntType lcm(IntType n, boost::call_traits<IntType>::param_type m);
and the current state is:
template <typename IntType> IntType lcm(IntType n, IntType m);
Okay.
Daniel
Jonathan
[SNIP Jonathan Turkanis and Daniel Krügler going back and forth on what the function signature for boost::lcm in <boost/rational.hpp> should be] I made a copy of the GCD and LCM functions in <boost/math/common_factor_ct.hpp> so other libraries can use that one and we can centralize improvements there. Maybe we should consider that. (That would mean removing the GCD and LCM currently in <boost/rational.hpp>.) I know Paul complained once that my code was too complicated (Has he seen the rest of Boost?!); maybe we can find some optimization. -- Daryle Walker Mac, Internet, and Video Game Junkie darylew AT hotmail DOT com
participants (2)
-
Daniel Krügler
-
Daryle Walker -
Jonathan Turkanis