=== What is your evaluation of the design? === The design seems to adequately capture important geometric concepts, such as dimensionality and coordinate systems, as well as numerical precision, etc. The 4-type overload for assign (center + radius for a sphere) deviates from the semantics of the 2-/3-arg versions, which I dislike. I have not reviewed any of the extensions, so perhaps there is a good reason doing this... Other than the fact that more people will probably use center+radius than 4-d polygons. Then again, assign is parameterized by the geometry type, so this is probably just an omission. Issues w/ units continue to nag at the back of my mind. However, specifying units for specific geometries (e.g. radians), and requiring consistency of units within a given geometry seems appropriate. Any design issues that would prevent strong robustness guarantees (without drastic refactoring) in the future should be identified and fixed prior to release. I *think* we are okay here, but have not put sufficient effort to make that call (because I don't need it). === What is your evaluation of the implementation? === I didn't have time to review the implementation, other than general poking-around in the code. I was happy to note that some of the functionality already works for k-dimensional euclidean geometries, even though it is technically not supported at the moment. And by k-dimensional, I mean k=4. :-) I didn't have time to push the limits. === What is your evaluation of the documentation? === In a nutshell, the documentation is inadequate, and IMO must be completed prior to release. I'm not sure whether it is best to require a mini-review, or some other process. A few anecdotes that illustrate the problem: The example code on the main page didn't compile, which isn't a problem in and of itself. However, it took an inspection of random code in the separate Examples section, and inductive reasoning to determine that I needed some additional includes beyond ggl.hpp to make the snippets work. I suggest a direct link to a working example program anywhere example code snippets are provided. I also had a bit of difficulty getting several algorithms to work, like union/intersection. The concepts were decently documented, however, translating the concepts to working code took some leg-work. The doxygen documentation seems incomplete. I expected a tab for functions, like union_inserter, but was only able to drill down to the functions by looking at the header docs. An algorithms page seems apropos. However, I couldn't find a page listing the available algorithms. I arrived at the union_inserter page via the doxygen-generated union.hpp header page. Average and worst case performance, as well as any known/suspected numerical issues or other bugs (such as the non-minimal result returned by union_inserter) IMO *must* be listed in the docs for each algorithm. This is the trade-off for not providing a 100% robustness guarantee. Even then, the pre/post conditions for algorithms must be specified, which seemed to be lacking from the docs. A note (personal opinion) regarding Doxygen documentation: Doxygen is best suited for a reference manual. For a User's/Programmer's guide, not so much. === What is your evaluation of the potential usefulness of the library? === This is a generally useful library across *many* domains. In particular, I am excited to make use of this library at work in our graphics/visualization infrastructure code, as well as our actual GIS features. In my academic research, I plan to make use of this library in statistical machine learning/data mining algorithms. However, it is a shame that the library lacks a first-class spatial index capability in it's present state (e.g. what is up for review). Even a relatively naive reference implementation using an R-tree would be useful to me. === Did you try to use the library? === Yup. * With what compiler? g++ (Ubuntu 4.4.1-4ubuntu8) 4.4.1 * Did you have any problems? The internal headers #include <ggl/blah.hpp>, which requires a -I<BOOST_ROOT>/boost directive, in addition to the usual -I required to find the boost headers. This needs to be fixed prior to release, such that only -I<BOOST_ROOT> is required. I am assuming that this is broken for the same reason that Barends chose not to put ggl in the boost namespace. Other than having to use the 'find' command to locate the appropriate headers for inclusion, and other documentation-related issues listed elsewhere in this review, I didn't experience much difficulty using the library. === How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? === On average, a couple hours a day, over the course of a week. Admittedly, I squandered too many hours justifying my own use case on the ML, which may have been better spent reviewing the implementation, and/or writing useful test cases. === Are you knowledgeable about the problem domain? === I have been a casual user (and occasional developer) of Computational Geometry and GIS algorithms at my current job for ~ 4 years. I have also taken a few (recent) graduate-level C.G. and numerical analysis courses. === Miscellany === The existing GGL mailing list(s) should probably be moved to boost/boost-users, and/or a boost-ggl list. Unless there is a compelling reason to maintain an external list (i.e. paid support, etc) , this will best serve the GGL users/developers. === Do you think the library should be accepted as a Boost library? === Yes, contingent on meeting a minimum level of documentation, as determined by the review manager and/or community. Thanks for putting so much work into this library! Jon