Re: [boost] [modularization] Modularizing Boost (modularization)

Subject: Re: [boost] [modularization] Modularizing Boost (modularization)
From: Edward Diener (eldiener_at_[hidden])
Date: 2013-10-21 18:03:29

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
>>> <boost/property_map/property_map.hpp> to a couple of files in the
>>> <boost/property_map/parallel/> 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
>> <boost/property_map/property_map.hpp>'. 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 <boost/property_map/parallel/...>.
>
>> 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.