FubarDevelopment / QuickGraph

Fork of https://quickgraph.codeplex.com/
Microsoft Public License
9 stars 2 forks source link

CP-65208: Implementation of OutEdges(TVertex v) for BidirectionalGraph is (I think) wrong #156

Open fubar-coder opened 6 years ago

fubar-coder commented 6 years ago

From unknown CodePlex user on Thursday, 01 September 2016 00:41:20

Here is the implementation for BidirectionalGraph :

        public IEnumerable<TEdge> OutEdges(TVertex v)
        {
            IEnumerable<TEdge> result;
            if (this.TryGetInEdges(v, out result))
                return result;
            else
                return Enumerable.Empty<TEdge>();
        }

I think it should not use the method "TryGetInEdges".

A symptom of this problem is that the algoritms have a weird behaviour. For exemple, is i have the following graph:

v1->v2
^    |
|    v
v4<-v3

and if I use the StronglyConnectedComponents algorithm on this graph, the algorithm will output that there is 4 components (we should have only one component). I think, this is because the StronglyConnectedComponents algorithm use the OutEdges method, and since it is wrong, the algorithm actually see the following graph:

|-----|   |-----|   |-----|    |-----|
v1<---|   v2<---|   v3<---|    v4<---|

(each vertex has a self-loop, and there is no other edge excepts these loops)

ktnr commented 5 years ago

As far as I can see this issue is fixed, the current implementation is:

public IEnumerable<TEdge> OutEdges(TVertex v)
{
    return this.vertexOutEdges[v];
}

I see you branched from another QuickGraph fork (https://github.com/YaccConstructor/QuickGraph/), which seems to be more active.

Can you tell me what the purpose of this project is? Also, if I wanted to contribute, should I contribute to that other fork? Unfortunately, the other team has not added the archived issues and discussions.

fubar-coder commented 5 years ago

@ktnr I suggest that you contribute to the other project instead of this one. I opened an issue explaining the current situation: #162.