Hi everyone, The review of Vicente Botet' Ratio library starts October 2nd and lasts until October 11th, 2010, unless an extension occurs. What is it? ======== Portable implementation of the C++0x Standard Library's compile-time rational arithmetic working on C++03 and C++0x compilers. See the current C++0x working draft at http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2010/n3126.pdf Boost.Ratio provides: * A class template, ratio, for specifying compile time rational constants such as 1/3 of a nanosecond or the number of inches per meter. ratio represents a compile time ratio of compile time constants with support for compile time arithmetic with overflow and division by zero protection * It provides a textual representation of boost::ratio<N, D> in the form of a std::basic_string. Other types such as boost::duration can use these strings to aid in their I/O. This library is header-only and is designed to be generic and useful in a lot of areas of math and numerics C++ programming, in particular it is absolutely needed for the proposed Boost.Chrono and Boost.Stopwatches libraries which are currently in the review queue. Getting the library ============= The latest version of this library may be downloaded from vault: http://www.boostpro.com/vault/index.php?action=downloadfile&filename=ratio.zip&directory=Math%20-%20Numerics& SVN: http://svn.boost.org/svn/boost/sandbox/chrono (directories boost/ratio and libs/ratio) and the docs may be viewed here html: http://svn.boost.org/svn/boost/sandbox/chrono/libs/ratio/doc/html/index.html pdf: http://svn.boost.org/svn/boost/sandbox/chrono/libs/ratio/doc/ratio.pdf You can get all these information also (which should avoid some long links) from http://svn.boost.org/trac/boost/wiki/LibrariesUnderConstruction#Boost.Ratio It is worth noting that the tests included in the zip file are relatively sparse. Vicente has an extensive suite of tests licensed under the LLVM license. He has posted a message on the Boost Developers mailing list regarding the license requirements for tests (See http://thread.gmane.org/gmane.comp.lib.boost.devel/208978), and is awaiting the outcome of that discussion before making the tests available. Writing a review ================ If you feel this is an interesting library, then please submit your review to the developer list (preferably), or to the review manager. Here are some questions you might want to answer in your review: - What is your evaluation of the design? - What is your evaluation of the implementation? - What is your evaluation of the documentation? - What is your evaluation of the potential usefulness of the library? - Did you try to use the library? With what compiler? Did you have any problems? - How much effort did you put into your evaluation? A glance? A quick - reading? In-depth study? - Are you knowledgeable about the problem domain? And finally, every review should answer this question: - Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion. Anthony Review Manager for the proposed Boost.Ratio library -- Author of C++ Concurrency in Action http://www.stdthread.co.uk/book/ just::thread C++0x thread library http://www.stdthread.co.uk Just Software Solutions Ltd http://www.justsoftwaresolutions.co.uk 15 Carrallack Mews, St Just, Cornwall, TR19 7UL, UK. Company No. 5478976
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Anthony Williams Sent: Monday, September 27, 2010 11:01 PM To: boost@lists.boost.org Cc: boost-announce@lists.boost.org Subject: [boost] [Review] Formal Review: Boost.Ratio
The review of Vicente Botet' Ratio library starts October 2nd and lasts until October 11th, 2010, unless an extension occurs. It is worth noting that the tests included in the zip file are relatively sparse. Vicente has an extensive suite of tests licensed under the LLVM license. He has posted a message on the Boost Developers mailing list regarding the license requirements for tests (See http://thread.gmane.org/gmane.comp.lib.boost.devel/208978), and is awaiting the outcome of that discussion before making the tests available.
If we decide we can't include the LLVM licensed tests, I think a small subset of tests will suffice - since we have the knowledge that it has been tested using non-Boost licensable test code. Anyone who really cares about more tests can get them and use them if their legal advisers permit. (I find the idea that seeing the LLVM code would make a work 'derivative' more than a bit far-fetched).
Here are some questions you might want to answer in your review: - What is your evaluation of the design? FWIW, I see no snags.
- What is your evaluation of the implementation? Seem mature.
- What is your evaluation of the documentation? OK, but needs proof reading by a non-author. Too many typos!
- What is your evaluation of the potential usefulness of the library? A 'must have' building block, and asap.
- Did you try to use the library? With what compiler? Did you have any problems? No.
- How much effort did you put into your evaluation? A quick re-reading
- Are you knowledgeable about the problem domain? Not much.
- Do you think the library should be accepted as a Boost library? Yes.
Paul --- Paul A. Bristow, Prizet Farmhouse, Kendal LA8 8AB UK +44 1539 561830 07714330204 pbristow@hetp.u-net.com
Hi Paul, ----- Original Message ----- From: "Paul A. Bristow" <pbristow@hetp.u-net.com> To: <boost@lists.boost.org> Sent: Tuesday, October 05, 2010 11:42 AM Subject: Re: [boost] [Review] Formal Review: Boost.Ratio
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of Anthony Williams Sent: Monday, September 27, 2010 11:01 PM To: boost@lists.boost.org Cc: boost-announce@lists.boost.org Subject: [boost] [Review] Formal Review: Boost.Ratio
Vicente has an extensive suite of tests licensed under the LLVM license. He has posted a message on the Boost Developers mailing list regarding the license requirements for tests (See http://thread.gmane.org/gmane.comp.lib.boost.devel/208978), and is awaiting the outcome of that discussion before making the tests available.
If we decide we can't include the LLVM licensed tests, I think a small subset of tests will suffice - since we have the knowledge that it has been tested using non-Boost licensable test code. Anyone who really cares about more tests can get them and use them if their legal advisers permit.
(I find the idea that seeing the LLVM code would make a work 'derivative' more than a bit far-fetched).
From the comments on the thread I can conclude that Boosters don't like to see files with LLVM license even if they are only for tests. If the library is accepted I will write a complete test suite under the BSL. The reason is that even if the library has been tested in some compilers and platforms I will need this complete test suite to ensure the library works correctly with other compiler and platforms. As I have no access to all these platforms, the single and simple way I see is to add them on Boost. In addition, the user will be able to check the library works as expected on its own environement without the need to down load another package.
- What is your evaluation of the documentation? OK, but needs proof reading by a non-author. Too many typos!
Paul, it would be great if you can do it if the library is accepted. Just, let me know.
- Do you think the library should be accepted as a Boost library? Yes.
Paul
Thanks a lot, Vicente
-----Original Message----- From: boost-bounces@lists.boost.org [mailto:boost-bounces@lists.boost.org] On Behalf Of vicente.botet Sent: Tuesday, October 05, 2010 5:20 PM To: boost@lists.boost.org Subject: Re: [boost] [Review] Formal Review: Boost.Ratio
- What is your evaluation of the documentation? OK, but needs proof reading by a non-author. Too many typos!
Paul, it would be great if you can do it if the library is accepted. Just, let me know.
OK - yes - mail me later - I set myself up for that didn't I ;-) Paul --- Paul A. Bristow, Prizet Farmhouse, Kendal LA8 8AB UK +44 1539 561830 07714330204 pbristow@hetp.u-net.com
On 09/28/2010 12:01 AM, Anthony Williams wrote:
[...]
Here are some questions you might want to answer in your review:
- What is your evaluation of the design?
Looks sound to me.
- What is your evaluation of the implementation?
The code looks very clean, except for a few #if 0, which I would rather have removed.
- What is your evaluation of the documentation?
Good to read, but needs consistency checks. For instance, the link See the source file test/ratio_test.cpp yields something completely different than the documentation... // //
- What is your evaluation of the potential usefulness of the library?
Very useful for scientific and engineering stuff. Wish I had had such a library at hand while studying physics... Also required for Chrono :-)
- Did you try to use the library? With what compiler? Did you have any problems?
As part of Chrono, yes. gcc 4.2.4 on Ubuntu 8.04, 64bit gcc 4.4.1 on Ubuntu 10.4 64bit No known problems.
- How much effort did you put into your evaluation? A glance? A quick - reading? In-depth study?
When trying to fix warnings in Chrono (while ratio was still part of Chrono), I spent several hours working through the code. For this review, I looked at the header files rather briefly and read the documentation.
- Are you knowledgeable about the problem domain?
A little bit...
And finally, every review should answer this question:
- Do you think the library should be accepted as a Boost library?
Yes. Thanks and regards, Roland
Hi Roland, ----- Original Message ----- From: "Roland Bock" <rbock@eudoxos.de> To: <boost@lists.boost.org> Sent: Tuesday, October 05, 2010 9:07 PM Subject: Re: [boost] [Review] Formal Review: Boost.Ratio
- What is your evaluation of the implementation?
The code looks very clean, except for a few #if 0, which I would rather have removed.
I guess you are talking of these kind of #if 0 #if 0 public: // The nested typedef type shall be a synonym for ratio<R1::num * R2::den, R2::num * R1::den>::type. typedef typename ratio<R1::num * R2::den, R1::den * R2::num>::type type; #else You are right that these kind of comments could be more troubling that helping. What do you think about removing the #if 0 and letting the comment as follows? // The nested typedef type is a synonym for ratio<R1::num * R2::den, R2::num * R1::den>::type that avoids overflows when possible. I can also remove it completly, as the documentation explain this problem widely.
- What is your evaluation of the documentation?
Good to read, but needs consistency checks. For instance, the link
See the source file test/ratio_test.cpp
yields something completely different than the documentation...
Hrr. I changed the name of the file quite lately. I will update it on the documentation. I guess that I will need to run the inspection tool to catch other bad links.
- Did you try to use the library? With what compiler? Did you have any problems?
As part of Chrono, yes.
gcc 4.2.4 on Ubuntu 8.04, 64bit gcc 4.4.1 on Ubuntu 10.4 64bit
No known problems.
I had no make test on 64 bits platforms. This is godd to know.
- How much effort did you put into your evaluation? A glance? A quick - reading? In-depth study?
When trying to fix warnings in Chrono (while ratio was still part of Chrono), I spent several hours working through the code. For this review, I looked at the header files rather briefly and read the documentation.
Thanks for helping me fixing warnings and for the many suggestion you did concerning the documentation.
And finally, every review should answer this question:
- Do you think the library should be accepted as a Boost library?
Yes.
And of course thanks for your positive review. Best, Vicente
"vicente.botet" <vicente.botet@wanadoo.fr> hat am 5. Oktober 2010 um 23:10 geschrieben:
- What is your evaluation of the implementation?
The code looks very clean, except for a few #if 0, which I would rather have removed.
I guess you are talking of these kind of #if 0
#if 0 public: // The nested typedef type shall be a synonym for ratio<R1::num * R2::den, R2::num * R1::den>::type. typedef typename ratio<R1::num * R2::den, R1::den * R2::num>::type type; #else
You are right that these kind of comments could be more troubling that helping. What do you think about removing the #if 0 and letting the comment as follows?
// The nested typedef type is a synonym for ratio<R1::num * R2::den, R2::num * R1::den>::type that avoids overflows when possible. This is fine, IMO
Thanks for helping me fixing warnings and for the many suggestion you did concerning the documentation.[...] And of course thanks for your positive review.
You're welcome! Thanks for the library :-) Regards, Roland
participants (5)
-
Anthony Williams -
Paul A. Bristow -
rbock＠eudoxos.de -
Roland Bock
-
vicente.botet