[ratio] metafunction ratio_subtract
Hi Beman, Hi Howard, With current implementation of the metafunction ratio_subtract I get the type from the test ratio_test.cpp User1::Distance d( User1::mile(110) ); User1::Time t( boost::chrono::hours(2) ); typeof (d/t) is User1::quantity<boost::ratio<1,-1>, boost::ratio<1,1> > So the following assignation do not works User1::Speed s = d / t; as User1::Speed is defined as typedef quantity<boost::ratio<1>, boost::ratio<0> > Time; // second typedef quantity<boost::ratio<0>, boost::ratio<1> > Distance; // meter typedef quantity<boost::ratio<-1>, boost::ratio<1> > Speed; // meter/second The code is a little bit triky, and I have not taken the time to understand why the resulting type is User1::quantity<boost::ratio<1,-1>, boost::ratio<1,1> > and not <boost::ratio<-1,1>, boost::ratio<1,1> > User1::quantity I have replaced the type by typedef ratio<R1::num * R2::den - R2::num * R1::den,R1::den * R2::den> aux_type; typedef ratio<aux_type::num ,aux_type::den > type; // get normalized type to follow the recommendation "The nested typedef type shall be a synonym for ratio<T1, T2> where T1 has the value R1::num *R2::den - R2::num * R1::den and T2 has the value R1::den * R2::den." and every thing is OK (See below the replaced code) I'm doing something wrong? Best, _____________________ Vicente Juan Botet Escribá template <class R1, class R2> struct ratio_subtract { private: #if 1 typedef ratio<R1::num * R2::den - R2::num * R1::den,R1::den * R2::den> aux_type; #else static const boost::intmax_t gcd_n1_n2 = detail::static_gcd<R1::num, R2::num>::value; static const boost::intmax_t gcd_d1_d2 = detail::static_gcd<R1::den, R2::den>::value;#endif public: //The nested typedef type shall be a synonym for ratio<T1, T2> where T1 has the value // R1::num *R2::den - R2::num * R1::den and T2 has the value R1::den * R2::den. #if 1 typedef ratio<aux_type::num ,aux_type::den > type; #else typedef typename ratio_multiply < ratio<gcd_n1_n2, R1::den / gcd_d1_d2>, ratio < detail::ll_sub < detail::ll_mul<R1::num / gcd_n1_n2, R2::den / gcd_d1_d2>::value, detail::ll_mul<R2::num / gcd_n1_n2, R1::den / gcd_d1_d2>::value >::value, R2::den > >::type type; #endif };
On Dec 6, 2009, at 3:43 PM, vicente.botet wrote:
Hi Beman, Hi Howard,
With current implementation of the metafunction ratio_subtract I get the type from the test ratio_test.cpp
User1::Distance d( User1::mile(110) ); User1::Time t( boost::chrono::hours(2) );
typeof (d/t) is User1::quantity<boost::ratio<1,-1>, boost::ratio<1,1> >
So the following assignation do not works
User1::Speed s = d / t;
as User1::Speed is defined as
typedef quantity<boost::ratio<1>, boost::ratio<0> > Time; // second typedef quantity<boost::ratio<0>, boost::ratio<1> > Distance; // meter typedef quantity<boost::ratio<-1>, boost::ratio<1> > Speed; // meter/second
The code is a little bit triky, and I have not taken the time to understand why the resulting type is User1::quantity<boost::ratio<1,-1>, boost::ratio<1,1> > and not <boost::ratio<-1,1>, boost::ratio<1,1> > User1::quantity
There must be a bug in the ratio formation of num and den. It should not be possible to form a ratio such that ratio::den is not positive. [ratio.ratio]/1 states that D shall not be 0. [ratio.ratio]/2 specifies that gcd operates on the absolute values of N and D (and so gcd must be positive). den is specified to be abs(D)/gcd, which must be positive.
I have replaced the type by
typedef ratio<R1::num * R2::den - R2::num * R1::den,R1::den * R2::den> aux_type; typedef ratio<aux_type::num ,aux_type::den > type; // get normalized type
to follow the recommendation "The nested typedef type shall be a synonym for ratio<T1, T2> where T1 has the value R1::num *R2::den - R2::num * R1::den and T2 has the value R1::den * R2::den."
and every thing is OK (See below the replaced code)
I'm doing something wrong?
No, you're doing something right by investigating this mystery. :-) I've lost the link to the code you're working with. Could you post it? The original implementation of ratio_subtract (thank you for including it below) is designed to be equivalent to your substitution, but less susceptible to overflow, by performing exact division as soon as possible (get integers as low as possible, as soon as possible). I'm not sure where your example of computing speed from distance and time involves subtraction. But I'm interested to help you get to the bottom of it. -Howard
Best, _____________________ Vicente Juan Botet Escribá
template <class R1, class R2> struct ratio_subtract { private: #if 1 typedef ratio<R1::num * R2::den - R2::num * R1::den,R1::den * R2::den> aux_type; #else static const boost::intmax_t gcd_n1_n2 = detail::static_gcd<R1::num, R2::num>::value; static const boost::intmax_t gcd_d1_d2 = detail::static_gcd<R1::den, R2::den>::value;#endif public: //The nested typedef type shall be a synonym for ratio<T1, T2> where T1 has the value // R1::num *R2::den - R2::num * R1::den and T2 has the value R1::den * R2::den. #if 1 typedef ratio<aux_type::num ,aux_type::den > type; #else typedef typename ratio_multiply < ratio<gcd_n1_n2, R1::den / gcd_d1_d2>, ratio < detail::ll_sub < detail::ll_mul<R1::num / gcd_n1_n2, R2::den / gcd_d1_d2>::value, detail::ll_mul<R2::num / gcd_n1_n2, R1::den / gcd_d1_d2>::value
::value, R2::den
::type type; #endif };
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
----- Original Message ----- From: "Howard Hinnant" <howard.hinnant@gmail.com> To: <boost@lists.boost.org> Sent: Monday, December 07, 2009 12:18 AM Subject: Re: [boost] [ratio] metafunction ratio_subtract On Dec 6, 2009, at 3:43 PM, vicente.botet wrote:
Hi Beman, Hi Howard,
With current implementation of the metafunction ratio_subtract I get the type from the test ratio_test.cpp
User1::Distance d( User1::mile(110) ); User1::Time t( boost::chrono::hours(2) );
typeof (d/t) is User1::quantity<boost::ratio<1,-1>, boost::ratio<1,1> >
So the following assignation do not works
User1::Speed s = d / t;
as User1::Speed is defined as
typedef quantity<boost::ratio<1>, boost::ratio<0> > Time; // second typedef quantity<boost::ratio<0>, boost::ratio<1> > Distance; // meter typedef quantity<boost::ratio<-1>, boost::ratio<1> > Speed; // meter/second
The code is a little bit triky, and I have not taken the time to understand why the resulting type is User1::quantity<boost::ratio<1,-1>, boost::ratio<1,1> > and not <boost::ratio<-1,1>, boost::ratio<1,1> > User1::quantity
There must be a bug in the ratio formation of num and den. It should not be possible to form a ratio such that ratio::den is not positive. [ratio.ratio]/1 states that D shall not be 0. [ratio.ratio]/2 specifies that gcd operates on the absolute values of N and D (and so gcd must be positive). den is specified to be abs(D)/gcd, which must be positive.
I have replaced the type by
typedef ratio<R1::num * R2::den - R2::num * R1::den,R1::den * R2::den> aux_type; typedef ratio<aux_type::num ,aux_type::den > type; // get normalized type
to follow the recommendation "The nested typedef type shall be a synonym for ratio<T1, T2> where T1 has the value R1::num *R2::den - R2::num * R1::den and T2 has the value R1::den * R2::den."
and every thing is OK (See below the replaced code)
I'm doing something wrong?
No, you're doing something right by investigating this mystery. :-) I've lost the link to the code you're working with. Could you post it? The original implementation of ratio_subtract (thank you for including it below) is designed to be equivalent to your substitution, but less susceptible to overflow, by performing exact division as soon as possible (get integers as low as possible, as soon as possible). I'm not sure where your example of computing speed from distance and time involves subtraction. But I'm interested to help you get to the bottom of it. -Howard _______________________ Hi, It is from the example related to Type-safe "physics" code interoperating with std::chrono::duration types and taking advantage of the std::ratio infrastructure and design philosophy. sandbox/chrono/libs/chrono/test/ratio_test.cpp The code I'm testing is derived from the one in the sandbox/chrono. I have modified something the implementation as the code didn't compiled on my environment (cygwin/gcc 3.4), most of them related to the fact that even on POSIX, the CLOCK_MONOTONIC is optionally defined. Please, could you check the unmodified code on the sandbox, if needed I can send you a patch this evening (french time). Thanks a lot, Vicente
On Dec 7, 2009, at 6:42 AM, vicente.botet wrote:
----- Original Message ----- From: "Howard Hinnant" <howard.hinnant@gmail.com> To: <boost@lists.boost.org> Sent: Monday, December 07, 2009 12:18 AM Subject: Re: [boost] [ratio] metafunction ratio_subtract
On Dec 6, 2009, at 3:43 PM, vicente.botet wrote:
Hi Beman, Hi Howard,
With current implementation of the metafunction ratio_subtract I get the type from the test ratio_test.cpp
User1::Distance d( User1::mile(110) ); User1::Time t( boost::chrono::hours(2) );
typeof (d/t) is User1::quantity<boost::ratio<1,-1>, boost::ratio<1,1> >
So the following assignation do not works
User1::Speed s = d / t;
as User1::Speed is defined as
typedef quantity<boost::ratio<1>, boost::ratio<0> > Time; // second typedef quantity<boost::ratio<0>, boost::ratio<1> > Distance; // meter typedef quantity<boost::ratio<-1>, boost::ratio<1> > Speed; // meter/second
The code is a little bit triky, and I have not taken the time to understand why the resulting type is User1::quantity<boost::ratio<1,-1>, boost::ratio<1,1> > and not <boost::ratio<-1,1>, boost::ratio<1,1> > User1::quantity
There must be a bug in the ratio formation of num and den. It should not be possible to form a ratio such that ratio::den is not positive. [ratio.ratio]/1 states that D shall not be 0. [ratio.ratio]/2 specifies that gcd operates on the absolute values of N and D (and so gcd must be positive). den is specified to be abs(D)/gcd, which must be positive.
I have replaced the type by
typedef ratio<R1::num * R2::den - R2::num * R1::den,R1::den * R2::den> aux_type; typedef ratio<aux_type::num ,aux_type::den > type; // get normalized type
to follow the recommendation "The nested typedef type shall be a synonym for ratio<T1, T2> where T1 has the value R1::num *R2::den - R2::num * R1::den and T2 has the value R1::den * R2::den."
and every thing is OK (See below the replaced code)
I'm doing something wrong?
No, you're doing something right by investigating this mystery. :-)
I've lost the link to the code you're working with. Could you post it?
The original implementation of ratio_subtract (thank you for including it below) is designed to be equivalent to your substitution, but less susceptible to overflow, by performing exact division as soon as possible (get integers as low as possible, as soon as possible).
I'm not sure where your example of computing speed from distance and time involves subtraction. But I'm interested to help you get to the bottom of it.
-Howard _______________________ Hi,
It is from the example related to Type-safe "physics" code interoperating with std::chrono::duration types and taking advantage of the std::ratio infrastructure and design philosophy. sandbox/chrono/libs/chrono/test/ratio_test.cpp
The code I'm testing is derived from the one in the sandbox/chrono. I have modified something the implementation as the code didn't compiled on my environment (cygwin/gcc 3.4), most of them related to the fact that even on POSIX, the CLOCK_MONOTONIC is optionally defined. Please, could you check the unmodified code on the sandbox, if needed I can send you a patch this evening (french time).
Ah... Hadn't seen that code in awhile. :-) You've uncovered a design flaw in ratio. And your ratio<N, D>::type is the fix! In terms of code: 1. type needs to be added to ratio: typedef ratio<num, den> type; 2. ratio_multiply should typedef to ratio::type, instead of to ratio: template <class _R1, class _R2> struct ratio_multiply { private: static const intmax_t __gcd_n1_d2 = __static_gcd<_R1::num, _R2::den>::value; static const intmax_t __gcd_d1_n2 = __static_gcd<_R1::den, _R2::num>::value; public: typedef typename ratio < __ll_mul<_R1::num / __gcd_n1_d2, _R2::num / __gcd_d1_n2>::value, __ll_mul<_R2::den / __gcd_n1_d2, _R1::den / __gcd_d1_n2>::value >::type type; }; 3. Ditto for ratio_divide. Points 2 and 3 will automatically fix ratio_add and ratio_subtract. What is happening: ratio<1, -1> was being given as the answer for ratio_subtract::type. This change gives instead ratio<1, -1>::type, which is ratio<-1, 1>. It is not invalid to create a ratio with a negative denominator. But that ratio will be normalized to have a positive denominator, which is a different type. The bug was not recognizing this important distinction (my bad). -Howard
----- Original Message ----- From: "Howard Hinnant" <howard.hinnant@gmail.com> To: <boost@lists.boost.org> Sent: Monday, December 07, 2009 8:11 PM Subject: Re: [boost] [ratio] metafunction ratio_subtract
On Dec 7, 2009, at 6:42 AM, vicente.botet wrote:
----- Original Message ----- From: "Howard Hinnant" <howard.hinnant@gmail.com> To: <boost@lists.boost.org> Sent: Monday, December 07, 2009 12:18 AM Subject: Re: [boost] [ratio] metafunction ratio_subtract
On Dec 6, 2009, at 3:43 PM, vicente.botet wrote:
Hi Beman, Hi Howard,
With current implementation of the metafunction ratio_subtract I get the type from the test ratio_test.cpp
User1::Distance d( User1::mile(110) ); User1::Time t( boost::chrono::hours(2) );
typeof (d/t) is User1::quantity<boost::ratio<1,-1>, boost::ratio<1,1> >
So the following assignation do not works
User1::Speed s = d / t;
as User1::Speed is defined as
typedef quantity<boost::ratio<1>, boost::ratio<0> > Time; // second typedef quantity<boost::ratio<0>, boost::ratio<1> > Distance; // meter typedef quantity<boost::ratio<-1>, boost::ratio<1> > Speed; // meter/second
The code is a little bit triky, and I have not taken the time to understand why the resulting type is User1::quantity<boost::ratio<1,-1>, boost::ratio<1,1> > and not <boost::ratio<-1,1>, boost::ratio<1,1> > User1::quantity
There must be a bug in the ratio formation of num and den. It should not be possible to form a ratio such that ratio::den is not positive. [ratio.ratio]/1 states that D shall not be 0. [ratio.ratio]/2 specifies that gcd operates on the absolute values of N and D (and so gcd must be positive). den is specified to be abs(D)/gcd, which must be positive.
I have replaced the type by
typedef ratio<R1::num * R2::den - R2::num * R1::den,R1::den * R2::den> aux_type; typedef ratio<aux_type::num ,aux_type::den > type; // get normalized type
to follow the recommendation "The nested typedef type shall be a synonym for ratio<T1, T2> where T1 has the value R1::num *R2::den - R2::num * R1::den and T2 has the value R1::den * R2::den."
and every thing is OK (See below the replaced code)
I'm doing something wrong?
No, you're doing something right by investigating this mystery. :-)
I've lost the link to the code you're working with. Could you post it?
The original implementation of ratio_subtract (thank you for including it below) is designed to be equivalent to your substitution, but less susceptible to overflow, by performing exact division as soon as possible (get integers as low as possible, as soon as possible).
I'm not sure where your example of computing speed from distance and time involves subtraction. But I'm interested to help you get to the bottom of it.
-Howard _______________________ Hi,
It is from the example related to Type-safe "physics" code interoperating with std::chrono::duration types and taking advantage of the std::ratio infrastructure and design philosophy. sandbox/chrono/libs/chrono/test/ratio_test.cpp
The code I'm testing is derived from the one in the sandbox/chrono. I have modified something the implementation as the code didn't compiled on my environment (cygwin/gcc 3.4), most of them related to the fact that even on POSIX, the CLOCK_MONOTONIC is optionally defined. Please, could you check the unmodified code on the sandbox, if needed I can send you a patch this evening (french time).
Ah... Hadn't seen that code in awhile. :-)
You've uncovered a design flaw in ratio. And your ratio<N, D>::type is the fix!
In terms of code:
1. type needs to be added to ratio:
typedef ratio<num, den> type;
2. ratio_multiply should typedef to ratio::type, instead of to ratio:
template <class _R1, class _R2> struct ratio_multiply { private: static const intmax_t __gcd_n1_d2 = __static_gcd<_R1::num, _R2::den>::value; static const intmax_t __gcd_d1_n2 = __static_gcd<_R1::den, _R2::num>::value; public: typedef typename ratio < __ll_mul<_R1::num / __gcd_n1_d2, _R2::num / __gcd_d1_n2>::value, __ll_mul<_R2::den / __gcd_n1_d2, _R1::den / __gcd_d1_n2>::value >::type type; };
3. Ditto for ratio_divide.
Points 2 and 3 will automatically fix ratio_add and ratio_subtract.
What is happening:
ratio<1, -1> was being given as the answer for ratio_subtract::type. This change gives instead ratio<1, -1>::type, which is ratio<-1, 1>. It is not invalid to create a ratio with a negative denominator. But that ratio will be normalized to have a positive denominator, which is a different type. The bug was not recognizing this important distinction (my bad).
OK, happy to see that my initial modification was in the good way. I'll generalize this on the Boost.Chrono code to explore in deep this idea. This will be useful to support the LWG issue http://home.roadrunner.com/~hinnant/issue_review/lwg-active.html#1281 Thanks, Vicente
participants (2)
-
Howard Hinnant -
vicente.botet