On Fri, Aug 10, 2012 at 2:23 PM, Dominick Vanthienen <
nick [dot] vanthienen [..] ...> wrote:

>
>
> On 08/10/2012 08:21 AM, Herman Bruyninckx wrote:
> > On Thu, 9 Aug 2012, Dominick Vanthienen wrote:
> >
> >> Dear developers,
> >>
> >> As mentioned in an earlier thread, we have a student doing some KDL
> development.
> >> On the agenda are KDL Tree and Chain. First I'll give the current
> >> situation and then I'll propose a change (and some topics to discuss).
> >
> > Thanks!!!
> >
> >> (See also 'conformity between chains and trees on the KDL wishlist:
> http://www.orocos.org/orocos/kdl-wiki/kdl-wishlist and
> http://www.orocos.org/kdl/examples#comment-32367)
> >> Feedback/ ideas on these changes are much appreciated!
> >
> > It would be nice if you could add some more lines of explanation of what
> > you suggest _exactly_ in the paragraphs below... I mean: I am often not
> > really certain about what and why you make suggestions.
> see proposed situation (I added some extra comments)
> >
> >> Nick
> >
> > Herman
> >
> >> current situation:
> >> (from http://people.mech.kuleuven.be/~rsmits/kdl/api/html/)
> >> TREE <=> CHAIN
> >> bool addSegment (const Segment &segment, const std::string &hook_name)
> <=> void addSegment (const Segment &segment)
> >> bool addChain (const Chain &chain, const std::string &hook_name)
> <=> void addChain (const Chain &chain)
> >> SegmentMap::const_iterator getSegment (const std::string &segment_name)
> const <=> const Segment & getSegment (unsigned int nr) const
> >> SegmentMap::const_iterator getRootSegment () const
> >> bool getChain (const std::string &chain_root, const std::string
> &chain_tip, Chain &chain) const
> >> private attribute: SegmentMap segments
> <=> public attribute: std::vector< Segment > segments
> >>
> >> proposed situation:
> >> *similar return types: void=> bool although this will always be true
> for a chain and therefore a bad idea
> for this I would like to have some opinions to do this change or not
> >> *get segment by name or number for a chain
> >> the search by name will be less efficient (log(n))
> >> *getRootSegment for chains (useful? this is just getSegment (0))
> >> *getChain for chains (to get a part of a chain)
> >> *public attribute segments => private attribute segments and function
> getSegments
> >> Situation now allows altering the vector of segments, which is not
> allowed
> >> *The big difference between a chain and a tree is the fact that a
> chain is an ordered list of segments while a tree doesn't contain this
> ordering. You could KDL::Tree for
> >> everything, but if you treat a chain as a tree, you can't ask for the
> end segment (since there is no strict ordering).
> here I would like to create a SegmentMap::const_iterator
> getNextBranchingSegment(const std::string &segment_name) that returns the
> segment where the tree has multiple
> branches (go up the tree until a segment with multiple childs) OR when the
> end is reached (segment without childs).
> If you have a chain in a KDL::Tree, you can find the end segment with it.
> This is practical when constructing trees from chains, where the chains
> are in KDL::Tree objects
>

If it is only about getting access to leaf segments, I think it is more
practical to store inside the tree a list of all leaf-segments. You can
than add some functions to get a leaf segment that will look inside this
list.

Ruben

> >>
> >> TREE <=> CHAIN
> >> bool addSegment (const Segment &segment, const std::string &hook_name)
> <=> bool addSegment (const Segment &segment)
> >> bool addChain (const Chain &chain, const std::string &hook_name)
> <=> bool addChain (const Chain &chain)
> >> SegmentMap::const_iterator getSegment (const std::string &segment_name)
> const <=> const Segment & getSegment (unsigned int nr) const
> >>
> AND const Segment & getSegment (const std::string
> &segment_name) const
> >>
> >> SegmentMap::const_iterator getRootSegment () const
> <=> const Segment & getRootSegment () const
> >> bool getChain (const std::string &chain_root, const std::string
> &chain_tip, Chain &chain) const <=> bool getChain (const std::string
> &chain_root, const std::string
> >> &chain_tip, Chain &chain)
> >> const SegmentMap & getSegments () const
> <=> const std::vector< Segment > & getSegments() const
> >>
> >
>
> --
> Orocos-Dev mailing list
> Orocos-Dev [..] ...
> http://lists.mech.kuleuven.be/mailman/listinfo/orocos-dev
>

On 08/10/2012 01:37 PM, Adolfo Rodríguez Tsouroukdissian wrote:
> The idea is a good one, but entails an API breakage. What's the stance on this, tick-tock it out (warn about deprecation in 1.x+1, deprecate in 1.x+2)?.

The question remains on how to do this properly, since it is unrelated to version numbers we can check.
In the end you would like to have one ROS package that can provide the old and new functionality to whoever asks for it.
Thus, compiling two libraries in the package, one with the old API and one with the new API, but then you have to state in the CMakeLists of packages using KDL which one of
the two you want to use.
Looks dirty to me.
Anyone a good solution, or just break API...

nick

On 08/10/2012 01:37 PM, Adolfo Rodríguez Tsouroukdissian wrote:
>
> > SegmentMap::const_iterator getSegment (const std::string
> &segment_name) const <=> const Segment & getSegment
> (unsigned int nr) const
> > AND const Segment & getSegment
> (const std::string &segment_name) const
>
>
> Here the return types for the getSegment (const std::string
> &segment_name) methods differ, as they expose the underlying data
> structures. I don't like it much, but I don't see any clean solution
> for KDL 1.x. In the long run, I'd like to see Chain and Tree share a
> common implementation, where Chain exists only for convenience reasons.
I am currently working on this part and I was wondering how to handle
the exceptional cases (e.g.: segment with the given name is not in the
chain).

I would suggest using a boolean return value to indicate whether the
segment is found with a variable by reference as parameter to return the
segment.

/bool getSegment (const std::string &segment_name, Segment&
returned_segment) const;/

For consistency reasons I would also refactor methods like /const
Segment& Chain::getSegment(unsigned int nr)const/, /const
std::vector<Segment> & getSegments() const/, /const Segment &
getRootSegment() const/ and their tree-counterparts to use this kind of
implementation.

I also noticed /const Segment& getSegment(unsigned int nr)const;/ does
not do any boundary checks. Has this choice been made for performance
reasons or should I implement the checks?

Best regards,
Wouter

On Fri, 10 Aug 2012, Adolfo Rodríguez Tsouroukdissian wrote:

[...]
> > proposed situation:
> > *similar return types: void=> bool although this will always be true for a chain
> and therefore a bad idea
>
> This improves chain-tree API consistency, so not a bad idea, even if it always evaluates to true
> for chains.
> > *get segment by name or number for a chain
> > the search by name will be less efficient (log(n))
>
> No problem here. Do you know use cases where these operations are called from
> performance-sensitive contexts, like inside control loops?.

The closest I get to such use cases is that of a humanoid robot, that
changes its topology while walking and/or when making contacts with the
environment or with itself (the latter most often via grasping an objects
with both hands). This brings the following extra challenges:
- the topology can become a _graph_ and not just a tree or serial chain.
Hence, solvers will have to be reconfigured _at runtime_, in order to
find a "spanning tree" over the graph that can be solved.
- these changes in topology could (will!) require runtime memory
allocation; don't expect this to be possible in hard realtime. I do not
think there is a hard realtime need for this:
- the cases like walking are so nominal that one should foresee the
corresponding trees/graphs at configuration time;
- the cases of topology changes due to task evolution should be dealt
with via state machine changes anyway, so only soft realtime is needed.
(One should foresee a "robust" ("compliant") wait state anyway, for the
case that a new tree must be made at runtime.)

Herman

On 08/10/2012 01:37 PM, Adolfo Rodríguez Tsouroukdissian wrote:
>
>
> On Fri, Aug 10, 2012 at 8:21 AM, Herman Bruyninckx <Herman [dot] Bruyninckx [..] ... Herman [dot] Bruyninckx [..] ...>> wrote:
>
> On Thu, 9 Aug 2012, Dominick Vanthienen wrote:
>
> > Dear developers,
> >
> > As mentioned in an earlier thread, we have a student doing some KDL development.
> > On the agenda are KDL Tree and Chain. First I'll give the current
> > situation and then I'll propose a change (and some topics to discuss).
>
> Thanks!!!
>
> > (See also 'conformity between chains and trees on the KDL wishlist: http://www.orocos.org/orocos/kdl-wiki/kdl-wishlist and
> http://www.orocos.org/kdl/examples#comment-32367)
> > Feedback/ ideas on these changes are much appreciated!
>
>
> Is this change planned for KDL 1.x?, and if so, should we expect no API breakages, but rather additions?.
some proposed changes will break API
I don't know which version it will be called :)
>
>
> It would be nice if you could add some more lines of explanation of what
> you suggest _exactly_ in the paragraphs below... I mean: I am often not
> really certain about what and why you make suggestions.
>
>
> The main motivation seems to be API consistency and satisfying the principle of least surprise: chains _are_ trees, so operations that are valid for trees should also be
> valid for chains, and exposed through a similar and consistent API. Opinions follow inline...
>
>
> > Nick
>
> Herman
>
> > current situation:
> > (from http://people.mech.kuleuven.be/~rsmits/kdl/api/html/ <http://people.mech.kuleuven.be/%7Ersmits/kdl/api/html />)
> > TREE <=> CHAIN
> > bool addSegment (const Segment &segment, const std::string &hook_name) <=> void addSegment (const Segment &segment)
> > bool addChain (const Chain &chain, const std::string &hook_name) <=> void addChain (const Chain &chain)
> > SegmentMap::const_iterator getSegment (const std::string &segment_name) const <=> const Segment & getSegment (unsigned int nr) const
> > SegmentMap::const_iterator getRootSegment () const
> > bool getChain (const std::string &chain_root, const std::string &chain_tip, Chain &chain) const
> > private attribute: SegmentMap segments <=> public attribute: std::vector< Segment > segments
> >
> > proposed situation:
> > *similar return types: void=> bool although this will always be true for a chain and therefore a bad idea
>
>
> This improves chain-tree API consistency, so not a bad idea, even if it always evaluates to true for chains.
indeed
(the downside is that this will introduce at the users side a check for a boolean that is always true (or a warning if you forget to do so))

>
> > *get segment by name or number for a chain
>
> > the search by name will be less efficient (log(n))
>
>
> No problem here. Do you know use cases where these operations are called from performance-sensitive contexts, like inside control loops?.
no, only at configuration time for my use cases
>
> For chains, getSegment returns a const Segment&, while for trees it returns a SegmentMap::const_iterator. There should be an alternative with the same signature for both
> (that of Tree, for generality).
if we change the chain segment vector to a map, there won't be much/a difference with a tree anymore :)
>
> > *getRootSegment for chains (useful? this is just getSegment (0))
>
>
> Yes again, for consistency reasons.
>
> > *getChain for chains (to get a part of a chain)
>
>
> OK
>
> > *public attribute segments => private attribute segments and function getSegments
>
> > Situation now allows altering the vector of segments, which is not allowed
>
>
> The idea is a good one, but entails an API breakage. What's the stance on this, tick-tock it out (warn about deprecation in 1.x+1, deprecate in 1.x+2)?.
tick tock is a possibility
>
> > *The big difference between a chain and a tree is the fact that a chain is an ordered list of segments while a tree doesn't contain this ordering. You could
> KDL::Tree for
> > everything, but if you treat a chain as a tree, you can't ask for the end segment (since there is no strict ordering).
>
>
> I agree that querying a chain end segment is convenient, but there's no big difference with trees. You can ask for the end segments (plural) of a tree, which correspond to
> all the leaf nodes. Easy access to tree end segments does not seem a great convenience gain, as we are --finally!-- seeing more and more use cases where chain/tree tips are
> no longer the kings of end effectors.
see other mail
>
> >
> > TREE <=> CHAIN
> > bool addSegment (const Segment &segment, const std::string &hook_name) <=> bool addSegment (const Segment &segment)
> > bool addChain (const Chain &chain, const std::string &hook_name) <=> bool addChain (const Chain &chain)
> > SegmentMap::const_iterator getSegment (const std::string &segment_name) const <=> const Segment & getSegment (unsigned int nr) const
> > AND const Segment & getSegment (const std::string &segment_name) const
>
>
> Here the return types for the getSegment (const std::string &segment_name) methods differ, as they expose the underlying data structures. I don't like it much, but I don't
> see any clean solution for KDL 1.x. In the long run, I'd like to see Chain and Tree share a common implementation, where Chain exists only for convenience reasons.
>
> >
> > SegmentMap::const_iterator getRootSegment () const <=> const Segment & getRootSegment () const
> > bool getChain (const std::string &chain_root, const std::string &chain_tip, Chain &chain) const <=> bool getChain (const std::string &chain_root, const std::string
> > &chain_tip, Chain &chain)
> > const SegmentMap & getSegments () const <=> const std::vector< Segment > & getSegments() const
>
>
> Previous argument about return types holds here as well.
>
> Cheers,
>
> Adolfo.

thanks for your input!
>
>
> >
>
> --
> --
> KU Leuven, Mechanical Eng., Mechatronics & Robotics Research Group
> <http://people.mech.kuleuven.be/~bruyninc > EURON Coordinator (European Robotics Research Network) <http://www.euron.org>
> Open Realtime Control Services <http://www.orocos.org>
> Associate Editor JOSER <http://www.joser.org>, IJRR <http://www.ijrr.org>
> --
> Orocos-Dev mailing list
> Orocos-Dev [..] ... <mailto:Orocos-Dev [..] ...>
> http://lists.mech.kuleuven.be/mailman/listinfo/orocos-dev
>
>
>
>
> --
> Adolfo Rodríguez Tsouroukdissian
> Robotics engineer
> adolfo [dot] rodriguez [..] ... <mailto:adolfo [dot] rodriguez [..] ...>
> http://www.pal-robotics.com <http://www.pal-robotics.com />
>
> PAL ROBOTICS S.L
> c/ Pujades 77-79, 4º4ª
> 08005 Barcelona, Spain.
> Tel. +34.93.414.53.47
> Fax.+34.93.209.11.09
>
> AVISO DE CONFIDENCIALIDAD: Este mensaje y sus documentos adjuntos, pueden contener información privilegiada y/o confidencial que está dirigida exclusivamente a su
> destinatario. Si usted recibe este mensaje y no es el destinatario indicado, o el empleado encargado de su entrega a dicha persona, por favor, notifíquelo inmediatamente y
> remita el mensaje original a la dirección de correo electrónico indicada. Cualquier copia, uso o distribución no autorizados de esta comunicación queda estrictamente prohibida.
>
> CONFIDENTIALITY NOTICE: This e-mail and the accompanying document(s) may contain confidential information which is privileged and intended only for the individual or entity
> to whom they are addressed. If you are not the intended recipient, you are hereby notified that any disclosure, copying, distribution or use of this e-mail and/or
> accompanying document(s) is strictly prohibited. If you have received this e-mail in error, please immediately notify the sender at the above e-mail address.