Re: [boost] [review] Dataflow Review starts today, September 1st

From: Stjepan Rajko (stipe_at_[hidden])
Date: 2008-09-02 00:42:33

Hi Vicente,

On Mon, Sep 1, 2008 at 4:23 PM, vicente.botet <vicente.botet_at_[hidden]> wrote:
> ----- Original Message ----- From: "Jaakko "Järvi"" <jarvi_at_[hidden]>
>
> Thanks for a so interesting library.
>

Thank you for your feedback!

>> This review focuses on the Dataflow.Signals layer of the library
>
> Do you mean that the Generic Support Layer is not reviewed?
>

Yes and no :-)  As something that the Dataflow.Signals layer is based
on, it certainly deserves scrutiny and I very much welcome feedback
about it.  But I don't believe that the Generic Support Layer in its
current state is really review-ready in its own accord.  For one, it
is not truly generic - it's design is heavily biased towards the needs
of Dataflow.Signals, and even though it has been successfully applied
to other dataflow frameworks, it leaves much to be desired.  Also, the
docs / examples aren't as complete as I think they should be to
facilitate a thorough review of that layer.

I do intend to keep working on that layer and would love to have a
formal review of it at some later point.

> For the moment just some questions and remarks on the documentation (typos,
> ...):
>
> * What does it means D in the following Port Requirement. Is it a Component,
> and the parameter (p) should not be a component instance?
> Name                        Expression                              Result
> Type    Semantics
> Get Default Port     get_default_port<D,M,T>(p) p Returns the port object.
>

Thanks for the catch - D is not documented on that page at all.  It
stands for a Direction type, and can be either dataflow::args::left or
dataflow::args::right.  For ports (when p is a port), get_default_port
is a identity function - it will just return the port.  If passed a
Component c, get_default_port<D,M,T>(c) will return the default port
of the component for that Direction and that Mechanism (its
ComponentTraits have a map of default ports, keyed by the Direction
and the Mechanism).  As default ports are usually accessed during a
binary operation (like connect), the Direction is deduced by the
components position in the expression (args::left if it is on the
left, args::right if on the right).  The Mechanism is usually
associated with the operation.  In Dataflow.Signals, the mechanism
associated with the connect operation is signals::connect_mechanism.
Take for example, in the context of Dataflow.Signals:

   c >>= p;

The underlying connect operation will do get_default_port<args::left,
M, T>(c) and get_default_port<args::right, M, T>(p) to get the actual
ports to be connected (where M=signals::connect_mechanism and
T=signals::tag).  If the component specifies a default port for
args::left and mechanism M, it will return it, and that is what will
get connected to port p, if possible.

> * Could you be more explict here? "ComplementedPorts are useful in
> situations where Port types are BinaryOperable in a one-to-one fashion (a
> pair of Port types are each other's port complements), or in a one-to-many
> fashion (a number of Port types have the same complement port). An example
> of the latter is Dataflow.Signals, where any signal of signature T has a
> complement port of type function<T>, and can therefore model
> ComplementedPort, but function<T> cannot because there are many signal types
> to which it can be connected. "
>

Maybe I can give you a concrete example of how ComplementedPorts can
be used.  A part of what the Dataflow.Blueprint layer does is take
Dataflow concepts and "turn them into" polymorphic runtime classes.
For example, there is the blueprint::port class, which is a
polymorphic base class that captures some Port functionality.  The
source is at:

http://svn.boost.org/svn/boost/sandbox/SOC/2007/signals/boost/dataflow/blueprint/port.hpp

There is also a class template port_t that inherits blueprint::port.
So now let's say I have two Port types - Source and Target.  And let's
say I want to have a run-time function connect as follows:

void connect(blueprint::port &producer, blueprint::port &consumer);

I would use it in a program something like this:

// the ports
Source source;
Target target;

// suppose we need type erasure
blueprint::port s = blueprint::port_t<Source>(source);
blueprint::port t = blueprint::port_t<Target>(target);

// we still want to connect
connect(s, t);

Now, we try to write the connect function.  In order for it to do a
dataflow connect, it needs to grab the underlying compile-time connect
function, which is templated on both Source and Target.
ComplementedPorts allow it to do that.  If Source ports can only
connect to Target ports, we can make Source a ComplementedPort with
Target as its complement.  Now, port_t<Source> has everything one
would need to know about instantiating the right operation to connect
Source to Target (because Target is Source's complement port type).
If you look at the port source file
I listed above, you will see that there is a complemented_port class
with the member function port_to_complement_connector() - that
function returns a function that knows how to connect the port to its
complement.

Returning to the implementation of void connect(blueprint::port
&producer, blueprint::port &consumer).  To do what it needs to do,
this function can do the following:
 1. check to see if producer is a complemented port (blueprint::port
has a member function is_complemented_port)
 2. if so, we can downcast it to complemented_port
 3. we can find out the type of its complement's type_info
 4. we can check to see whether consumer is of that type
 5. if all went well, get the port_to_complement_connector from
producer, and apply it to (producer, consumer).  Now they are
connected.

I'm not sure whether this description helped at all.  Basically, if
PortTypeA only plays with PortTypeB, we can make it a
ComplementedPort, which basically says "PortTypeA only plays with
PortTypeB", and that piece of information can make things easier.

> * In VectorPort Refines, the link PortVevtor is invalid, and PortVector is
> not defined previously.
>

Oops

> * The names PortVector and VectorPort are confusing.
>

I agree 100%.  For the time being, you can think of PortVector as a
"vector of ports", and "VectorPort" as "a vector of ports that is
itself a Port".  I have a redesign of the Generic Support Layer in
mind, and this nomenclature will go away.

> * There is an error in PortVector Requirements
> PortVector Traits traits_of<>::type PVT The ComponentTraits of the
> component.
> Something missing between <> and the semantic do not conforms to the name
>

Yeah that should be  traits_of<PV>::type, and the description should
be "The PortVectorTraits" of the PortVector.

> * Need to add in the notation What is M and c in PortVector Requirements.
> GetPort get_port_c<M, I>(c) Returns the I'th PortVectorTraits exposed by C
> The name get_port_c don not conforms very well to the semantics. Is
> something wrong?
>

Yikes, that page has lots of errors.

> * What does M stands for in Component Requirements
> GetComponentPort get_port<M, I>(c) Returns the I'th Port exposed by C
>

That should be get_port<I, T>(c), where T is a tag

> * In ComponentOperable the use of C seems more convenient than P?
>

Another messed up page...

> * In "Setting up a producer Port and a consumer Port for VTK
> Now that we have the mechanism"
> What mechanism stands for (maybe this should be tag)?
>

Yep, should be tag.  At one point, Mechanism became Tag, and then I
brought back Mechanism for a different purpose.  Recipe for disaster.

Thanks for all the error-catching.  As you see, there are some good
reasons behind me thinking that the Generic Support Layer is not ready
for review :-)   But I'm glad you're giving it a look - I will give
the docs for that layer a pass tomorrow and hopefully make them a
little bit more up to snuff.

Kind regards,

Stjepan