Factor graph wrong edge creation

[kris-singh]

Factor graph model add_edge presently accepts both (nodes, nodes) or (factors, factors) as input and creates a edge between them. A factor graph is a bipartite graph and should only have edges between (nodes, factors). Hence i think we need to change this. I implemented this but some test in the file test_factor_model.py have test cases like self.graph.add_edges_from([('a', 'phi1'), ('b', 'phi1'),'b', 'phi2'), ('c', 'phi2')]) which i don't get if we are adding a edge from a->phi1 why are passing a 'phi1' this actually create a edge between 'a' -> 'phi1' instead of creating a edge between node a and factor phi1

[kris-singh]

Should i correct these test and create a PR

[raghavg7796]

@kris-singh I think that supplying add_edge_from([('a', 'b')]) will create two nodes 'a' and 'b' if not already present. add_edge_from is in base class and for Factor Graph it will just add two nodes but not create a factor out of them and create an edge between them and for other models, it will also create an edge between them.

[raghavg7796]

@ankurankan Please confirm.

[kris-singh]

@raghavg7796 it should not create a edge between nodes as a factor graph is bipartite graph. So we need to add a check like if type(u) == type(v): raise ValueError('............something"). I have made the changes will

[raghavg7796]

@kris-singh Yes, and check_model() will give an error if you create an edge between nodes. Check_model does that stuff to ensure bipartiteness of the Factor Graph.

[ankurankan]

@raghavg7796 @kris-singh I checked the doctests and there it seems to be correct. But the tests seem to be using strings. @abinashpanda Do you remember how this was supposed to be ?

Regarding the bipartite thing, check_model is supposed to be handling that. We don't put constraints on the models while creating it.