[Review Results] Boost.Polygon library accepted into boost
Dear Boosters, [first of all allow me to apology for not doing this before. My small consulting bussines suddenly grew out of proportion and now is way over my head, keeping me working/managing 16 hours a day] I am pleased to announce that the Boost.Polygon library from Lucanus Simonson has been accepted into boost provided some of the most critical concerns, detailed below, are addressed first. First of all I would like to thank Lucanus and Intel Corporation for making this work available to all C++ developers around the world. I would also like to thank all the reviewers that participated (in no particular order nor degree of participation) Thomas Klimpel Frank Mori Hess Barend Gehrels Andreas Fabri Jeffrey Hellrung Tim Keitt Markus Werle Paul A. Bristow Robert Stewart Mathias Gaunard Michael Fawcett Steven Watanabe Joachim Faulhaber John Bytheway Sebastian Redl Mika Heiskanen John Phillips Kai Benndorf Hartmut Kaiser Arash Partow Maurizio Vitale Brandon Kohn David Abrahams Gordon Woodhull Daniel James John Maddock Tom Brinkman Bo Persson Mateusz Loskot Christian Henning Jean-Sebastien Stoezel The library had 6 yes votes and 4 no votes. Those who voted no made the following major complains and remarks (they are numbered as I will refer to them back later): Barend Gehrels: (1) According to his benchmarks, intersection algorithms are unnecesarily slow as if something where fundamentally wrong in the design and/or implementation. AFAICT the implication has not been proved or disproved so far, hence I consider it a red herring and cannot take it as a reason for rejecting the library, but it is important to look at it in detail. (2) As a major complain, the library is unusable in those domains where floating point support is a requirement (like GIS). (3) The library should contain some basic polygon geometry algorithms such as convex hull and centriod Phil Endecott: (4) There is no concept checking in the code so it is difficult to adapt. (5) There where too many warnings and build errors (but Luke fixed these) (6a) The documentation needs a better overview of operations supported by the concetps. (6b) It also needs the details on the algorithms complexities. (6c) It also misses the numerical requirements for the coordinate type. (6d) And tutorials. (7) Some operators are ambiguous and it would be better to keep just one of each "flavor". (8) The polygon_data template class is parameterized by a coordiante type rather than a point type. This makes it useless for integration with existing geometric data types Michael Fawcett: (5), (6a), (6d) (9) T9he API should provide versions using output iterators in addition to output container references. (10) Theoreticall the library would be usable with a user defined coordinate type (metting certain requirements now undocumented), for example a fixed point data type, but some details in the internal design and implementation prevents this. (11) As a major complain, the library is unusable in those domains where floating point support is a requirement (like GIS). Hartmut Kaiser: (11), (1) Fernando Cacciola: (12) The name of the library should change to better indicate that is restricted to Integral coordinates and two-dimensions. (13) The documentation must contain a "credits" section acknowledging all the people that participated in the review. For the library to be effectively accepted, points (5),(6a-d),(10), (12) and (13) should properly addressed ((5) is already done AFAICT) It would be quite significant if point (1) where looked into. Once again I thank Lucanus, Intel Corporation and all the reviwers. Best regards -- Fernando Cacciola SciSoft Consulting, Founder http://www.scisoft-consulting.com
Fernando Cacciola wrote:
(10) Theoreticall the library would be usable with a user defined coordinate type (metting certain requirements now undocumented), for example a fixed point data type, but some details in the internal design and implementation prevents this.
Can someone point me in the direction of a good fixed point library with reasonable license that I can use to test support of fixed point types and add the coordinate_concept mapping code of a fixed point datatype to the examples section of the documentation? Thanks, Luke
"Luke" == Simonson, Lucanus J <lucanus.j.simonson@intel.com> writes:
Luke> Fernando Cacciola wrote: >> (10) Theoreticall the library would be usable with a user defined >> coordinate type (metting certain requirements now undocumented), >> for example a fixed point data type, but some details in the >> internal design and implementation prevents this. Luke> Can someone point me in the direction of a good fixed point Luke> library with reasonable license that I can use to test support Luke> of fixed point types and add the coordinate_concept mapping Luke> code of a fixed point datatype to the examples section of the Luke> documentation? SystemC (www.systemc.org) has a numeric datatype library. Need to include/link in everything, but datatypes can be used on their own. Mentor's datatypes at http://www.mentor.com/products/esl/high_level_synthesis/ac_datatypes. I'd say neither license is acceptable for boost, but they should be both good enough for testing. Tests could be included, but people running them would have to get those libraries by themselves. I'd go for the Mentor library, if I had to chose. OTH, Maurizio --
Fernando Cacciola wrote:
(5) There where too many warnings and build errors (but Luke fixed these)
Done
(6a) The documentation needs a better overview of operations supported by the concetps.
Improved, there is more general discussion of what operations are provided by the library on the main page.
(6b) It also needs the details on the algorithms complexities.
Done, all interfaces documented include algorithmic complexity when applicable.
(6c) It also misses the numerical requirements for the coordinate type.
Done
(6d) And tutorials.
Not Done.
(10) Theoreticall the library would be usable with a user defined coordinate type (metting certain requirements now undocumented), for example a fixed point data type, but some details in the internal design and implementation prevents this.
Done, I adapted Algorithmic C ac_int<128> successfully.
(12) The name of the library should change to better indicate that is restricted to Integral coordinates and two-dimensions.
We settled on Boost.Polygon as a compromise.
(13) The documentation must contain a "credits" section acknowledging all the people that participated in the review.
Done
For the library to be effectively accepted, points (5),(6a-d),(10), (12) and (13) should properly addressed ((5) is already done AFAICT)
It would be quite significant if point (1) where looked into.
Related to performance concerns (1) I improved the line segment intersection algorithm, proved the comparison was a red herring and greatly improved the efficiency of the library's use of gmp or equivalent high precision numerical data types. I also have boost build and test working with my unit test and the unit test passing when compiled and run through bjam. The unit tests compile cleanly with -pedantic now. It looks like the only issue not yet addressed is (6d). I'd like to start migrating the library from sandbox to trunk and preparing for release. I'm not sure how to proceed at this point. Is there a contact within boost (Hartmut or Dave) that I can work with to get the library transitioned to the trunk and scheduled for release? At this stage I feel confident that I can write a tutorial in time for whatever upcoming release of boost the library is scheduled to be bundled into. Thanks, Luke
On 7 April 2010 22:28, Simonson, Lucanus J <lucanus.j.simonson@intel.com> wrote:
It looks like the only issue not yet addressed is (6d). I'd like to start migrating the library from sandbox to trunk and preparing for release. I'm not sure how to proceed at this point. Is there a contact within boost (Hartmut or Dave) that I can work with to get the library transitioned to the trunk and scheduled for release? At this stage I feel confident that I can write a tutorial in time for whatever upcoming release of boost the library is scheduled to be bundled into.
I don't think this is set down anywhere, but this is roughly how it goes. I'm sure someone will correct me if I get anything wrong. Once your review manager has given you the go ahead you can add it to trunk. It's a good idea to do this as soon as possible so you can add it to the regression tests. When you feel you're ready for release you should contact the release managers (which I think is best done on the development list, so other people know what's going on, although further release discussion is usually off list to reduce noise). We have a checklist that your library needs to meet before it can be merged to the release branch: https://svn.boost.org/trac/boost/wiki/ReleasePractices/ManagerCheckList#NewL... Some items are things that we take care of (such as adding to the website). You can be included in a release if your library is ready by the deadline for new libraries (usually 7 weeks before release, in the past we've been lenient if we cause a library to miss the deadline but I think that depends on the nature of the library). Daniel
participants (4)
-
Daniel James -
Fernando Cacciola -
Maurizio Vitale -
Simonson, Lucanus J