On 10/21/2013 4:48 PM, Jeremiah Willcock wrote:
On Mon, 21 Oct 2013, Edward Diener wrote:
On 10/21/2013 1:29 PM, Jeremiah Willcock wrote:
On Sat, 19 Oct 2013, Edward Diener wrote:
On 10/19/2013 4:39 PM, Jeremiah Willcock wrote:
On Sat, 19 Oct 2013, Edward Diener wrote:
> I had thought going the other way would be better: the files in > Boost.Graph that property map code depends on don't themselves use > much > other graph code. Thus, they can be moved to Boost.PropertyMap > without > too much work, and that keeps all of the property map types that are > there now in that library. There are other property map types > (sequential and parallel) in Boost.Graph that should likely also be > moved even though they don't need to be for dependency reasons.
If you do that it looks like you are then keeping a dependency for property_map on multi_index, serialization, and optional. I am not saying this is wrong if you feel that a distributed_property_map really should be part of property_map, since those addititional libraries are very useful for Boost libraries. I just wanted to point that out. Whereas the more normal non-distributed property map does not need to use those libraries AFAICS.
Another option is to have distributed_property_map be a separate libray of its own. More work, obviously, but this would create a more minimal set of dependencies for property_map itself.
A separate library might make more sense, actually. Another option would be for property_map/parallel to be treated as a separate library without being moved in the Boost tree.
I think that would be fine actaully.
For that to work you will need to move the specializations of distributed_property_map from their places in property_map.hpp and vector_property_map.hpp to the property_map parallel directory as separate files with theier own names, as well as remove the header file inclusions for the parallel subdirectory in the otiginal files. I think that would satisfy the end-user who wanted to use property_map without having to drag in those dependencies for distributed_property_map.
Also the distributed_property_map should probably then be documented as part of property_map rather than as part of graph, although graph could have the appropriate link to that documentation.
Similarly tests for distrubuted_property_map should be part of that implementation rather than graph, and the correct header files referenced.
Right now, there is a #include from
to a couple of files in the directory, conditioned on a macro being defined. Now that the parallel subdirectory will be treated as a separate library, this is likely to count as a dependency (which will be circular). Should I keep that in for compatibility? The code in property_map.hpp starting with '#ifdef BOOST_GRAPH_USE_MPI' to the end of the #ifdef should all be moved to the parallel subdirectory IMO. It can be called, let's say, distributed_iterator_property_map.hpp and of course will '#include
'. This moves the dependency out of property_map.hpp, so that end-users of the various property_map types are not bringing in the distributed property_map headers and code. That is what I did, but left the #ifdef in with its body as just a #include for the version in
. Similarly in vector_property_map.hpp where the '#ifdef BOOST_GRAPH_USE_MPI' code can be moved to a file in the parallel subdirectory, called perhaps distributed_vector_property_map.hpp.
Same comment here.
The idea is that anyone using the various non-distributed property maps, which are the majority of property map usages, are not bringing in the distributed property map code and if an end-user wants a distributed property map version he needs to directly include the correct header file from the parallel directory.
Doesn't this seem like the way it should work to you ?
It is preferable in my opinion, but would break compatibility with old code that assumes that the sequential code pulls in the parallel code.
Yes, I agree it would. But I think it was a mistake to pull in the parallel code, with all of its dependencies, when just using the sequential property_map code. Even though you have reduced the dependencies of the parallel code by removing the dependency on the graph library there are still other dependencies which anyone using the sequential code should not have. This will be even more important in a modularized Boost system. The fix for breaking the old code is very easy, just including the correct header file from the parallel directory. If you want to make it even easier just create forwarding headers in the main property_map directory to bring in the needed headers in the parallel directory. A note for the 1.56 release can tell any users of the parallel code in property_map what to do in regards to their header file inclusion, and a message in the mailing list(s) can tell the same users who are using 'trunk' what to do.