Re: [boost] Updating the Boost Review Process Was: [GGL] Bost.Polygon (GTL) vs GGL - rationale

Subject: Re: [boost] Updating the Boost Review Process Was: [GGL] Bost.Polygon (GTL) vs GGL - rationale
From: Simonson, Lucanus J (lucanus.j.simonson_at_[hidden])
Date: 2009-11-18 22:09:55

Phil Endecott wrote:
> I would really like a single common set of basic concepts to code to.
> As I wrote in my GGL review, there are gratuitous differences in
> terminology like "within" vs. "contains".  I have seen no discussion
> of whether we think this should be fixed, can be fixed, will be fixed
> etc.  Let alone who should "cede ground" in order to arrive at a
> compromise, or any technical discussion of how e.g. the point concepts
> could inter-operate.
> 
> Personally I'm totally unmotivated to contribute to "Boost.Geometry"
> if 
> I have to either do everything twice or gamble on which one is going
> to "win" in the end.
> 
> I would really like to see some discussion of this before this review
> ends, though sadly I will be going away tomorrow and may not be able
> to 
> take part in much of the remaining discussion.

Phil, I understand both your high expectations and your disappointment.  If you look back in the archives you will find discussion between myself and Barend where we did go into the technical details of what the semantics of geometry concepts ought to be and how the concepts should be implemented.  If we had been able to agree early on we could have collaborated.  Barend generally was not receptive to my criticism of his designs for his concepts in which I pointed out how what I was doing differed and *why* I was doing it differently.

We arrive now at a state where his polygon concept:

Requires that the user maintain an invariant positive winding direction, either clockwise or counter clockwise.
Requires that the user maintain the invariant that the first and last point in a "ring" are identical.
Requires that the user provide mutable access to an object with STL container interfaces to access the holes.
Has two different ways to add points to a ring, which is needlessly confusing.
Requires random access iterators (I think) for a ring as well as linestring, or was it just linestring?

My polygon_with_holes concept on the other hand:

Does not require the user to maintain an invariant positive winding, but allows the user to specify that they do so and which orientation it has.  If the user does not maintain the invariant I check the winding at runtime when that information is needed.
Does not require that the user enforce either "open" or "closed" invariant with regard to whether the first and last vertex are identical.  I check this case in all algorithms and handle either equally well.
Use iterator pair for getting and setting points of polygons as well as holes.
Require forward const iterators only.

My polygon_with_holes concept is more generic than Barend's polygon concept in every point, and I discussed this with him several times over the course of years.  Now in the review of his library we have the ability to switch invariant winding direction half implemented, still no plan to support checking the winding direction at runtime, a plan to support both open and closed semantics for the last vertex (but nothing implemented) with conflicting information about this in the documentation and a polygon concept that can only adapt legacy polygon types if the user declares a proxy class that fakes an STL container interface for access to the holes.

Many of these policies and invariants are a convenience to the library author, and it is hard to retrofit support for not requiring invariants from the data type once a large code base of algorithms has been developed that rely on them.  It would have been better if Barend had adopted more generic polygon concept semantics earlier, but he seemed to regard the extra work entailed to make his polygon concept more generic not worth the effort.  I'd already put in the work to make my algorithms invariant to winding direction and open/closed as well as to define a polygon concept interface that was easy to adapt for legacy types.

What Barend never accepted was that some polygon data types enforce each of the four possible combinations of clockwise/counterclockwise open/closed invariant and that you can't call his correct() function on such a data type because it will just see the wrong winding direction fed back in and revert it to the original and drop the redundant last vertex to save memory.  I know this because Intel has a large number of legacy polygon data types and I designed my concepts to easily adapt all of them.  Why would I throw out support for 3/4 of them?  His offer to allow me to join his project, let him drive design decisions and throw away my own concepts hierarchy was not so temping early on and particularly un-enticing later when it came with a threat that he would release his benchmark results during the review of my library if I didn't call off my review.

> Changing the subject slightly, I have also wondered over the last few
> days about acceptance criteria.  Many of Barend's emails mention
> things 
> that have not yet been implemented or are planned.  I wonder whether
> we 
> are, or should be, judging the library in terms of what has been
> submitted for review, or what we believe that the authors will
> eventually deliver?  Based on previous proposals where "fully formed"
> libraries have been presented, and some comments from review managers
> / wizards(?) that "the library being reviewed is the one submitted", I
> have always assumed the former.  In this case, I think that some
> reviewers are assuming the latter.

My concern is that many reviewers are having trouble distinguishing between what is actually submitted and what is a planned feature.  I get confused myself.

Regards,
Luke