Hi Vicente, On Mon, Sep 1, 2008 at 4:23 PM, vicente.botet <vicente.botet@wanadoo.fr> wrote:
----- Original Message ----- From: "Jaakko "Järvi"" <jarvi@cs.tamu.edu>
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/bluep... 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