nenb / blossalg

Construct a maximum matching on a graph using the blossom algorithm.
MIT License
2 stars 1 forks source link

state is not reset when calling compute_max_matching multiple times #6

Open danielquintao opened 2 years ago

danielquintao commented 2 years ago

Hello

This may not be a bug if the package is supposed to be used only from CLI, but I tried to adapt it to import it and use its functions, but I have the strange result that the vertices' indices are "accumulated" each time I call the same function:

{0: 1, 1: 0, 2: 3, 3: 2, 4: 5, 5: 4}
{6: 7, 7: 6, 8: 9, 9: 8, 10: 11, 11: 10}
{12: 13, 13: 12, 14: 15, 15: 14, 16: 17, 17: 16}

The code (inspired from your __main__.py):

import csv
import blossom.blossom as blossom

def read_infile(infile):
    node_array = []
    with open(infile) as csvfile:
        for row in csv.reader(csvfile, delimiter=str(",")):
            neighbours = [idx for idx, row in enumerate(row) if row == "1"]
            node_array.append(neighbours)
    if len(node_array) == 0:
        raise SystemExit("Empty graph. Please supply a valid graph.")
    return node_array

def compute_max_matching(node_array):
    # Create node instances, fill node neighbours
    nodelist = [blossom.Node() for _ in range(len(node_array))]
    for idx, node in enumerate(node_array):
        nodelist[idx].neighbors = [nodelist[node] for node in node]

    # Create graph instance, construct graph
    graph = blossom.Graph()
    graph.nodes = {node.name: node for node in nodelist}
    graph.compute_edges()

    # Compute maximum matching
    graph.find_max_matching()
    dict = graph.create_matching_dict()

    return dict

if __name__ == "__main__":

    node_array = read_infile("../data/input.csv")
    matched_dict = compute_max_matching(node_array)
    print(matched_dict)
    matched_dict = compute_max_matching(node_array)
    print(matched_dict)
    matched_dict = compute_max_matching(node_array)
    print(matched_dict)

The test file "../data/input.csv":

0,1,0,0,0,0
1,0,0,0,0,0
0,0,0,1,0,0
0,0,1,0,1,0
0,0,0,1,0,1
0,0,0,0,1,0

If the package should be only a CLI tool, sorry for opening this issue, and thank you for your attention in either case

nenb commented 2 years ago

Hello!

Thanks for opening this issue (and sorry about such a delay in responding).

I think you have summarised the situation perfectly! The package was initially designed only as a CLI tool (as you said). If it's not used via the CLI, there will likely be an issue here (if previous Node objects already exist, then they will affect the label/name of new nodes).

But, arguably it's more natural to import the package (as you have done), rather than use it as a CLI tool. Unfortunately I don't have much time to work on this at the moment, but I would be very happy to accept a PR. If you are interested, feel free to add new functionality in whatever way you think best (your analysis so far has been great).

Thanks for the interest!

danielquintao commented 2 years ago

Thank you for your answer , I am happy for understanding the cause of the behaviour now. Since the intended use is CLI-based, feel free to close the issue if you prefer (or to leave it open if you want this new functionality to be added one day). I am unable to say if I will contribute to the package but, if one day it turns to be the case, you will see a PR

Thank you again, your package was very useful in a project of mine (I dealed with the cumulated indexes using a "manual" solution but it works!)