psycharo-zz / factor-graph

matlab/c++ factor graph framework
32 stars 8 forks source link

no double declaration of edges #19

Closed ghost closed 12 years ago

ghost commented 12 years ago

In kalman_example, we write

>> xin.setDest(e);
>> e.setConnections(xin, a, b);

There is something unsatisfying about this in the sense that the connection xin-to-e is declared twice. Actually, all the connections are declared twice in kalman_example.

Is it possible to declare connections just once by code like this (or alike):

>> link1 = ffg.Link( xin, e ); % declares link from xin to e
>> link2 = ffg.Link( e, b ): % equ node to add node

This has the advantage that the direction of the edge is clear ( edge = ffg.Link(FromNode, ToNode) ), and that the variables in the network (ie, the links) can be assigned to a MATLAB variable (link1, link2, etc).

Note sure how this would work out at the back-end but for the user the network consists of node objects and link objects. FIrst we declare the node objects and then the link objects, eg,

>> xin = ffg.EvidenceNode( 'property', 'value');
>> e = ffg.EqualityNode(); 
>> link1 = ffg.Link( xin, e ); % connects xin to e
psycharo-zz commented 12 years ago

I got your point, what I don't like is that the Link is not useful by itself, so it might be better to create e.g. ffg.Network, with a method addEdge(a, b) or smth. This approach should also be useful when implementing loopy mode, the network will be in charge of message scheduling

ghost commented 12 years ago

OK, this addEdge(a,b) idea works fine. Problem solved.

One question I have. What does the network class do other than group a few functions. Do you need a class for that? Why not have a toolbox with folder structure +ffg/core and put the functions makeStep, addEdge, setSchedule in the core folder. This is core functionality of the toolbox. Yuou can call these fucntions by `ffg.addEdge

psycharo-zz commented 12 years ago

it verifies (for now very basically) whether the schedule is correct or not. also the schedule itself should be stored somewhere.

ghost commented 12 years ago

ok, sounds good, thx