-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updating topological grouping algorithm #125
Comments
@metincansiper Do you think this is needed ? Topological grouping is an operation that we run on graphs with at most several tens of nodes, so efficiency shouldn't be such a big issue I think. I wouldn't play with what works :) |
@ugurdogrusoz I see that the sample sif graphs are small. However, there is a feature to import sif graphs. That is why I thought that there could be a possibility of having larger sif graphs. However, if you think that the imported graphs would not be large anyways then there should not be any need for optimization here. Or if you think that it would not worth playing with a currently working feature anyways, then we can skip it :) |
What do you think @ozgunbabur? |
Well, I haven't had any problem with the current implementation. When I load a relatively large SIF to Newt (loads in about 10 seconds), and click to use topology grouping, it takes about 2-3 seconds to do it. IMO, a faster implementation will not hurt, so if you have time for that Metin, I think it would be nice. But it is not critical at all. |
@ozgunbabur great then I can work on it in an availability if that sounds good to @ugurdogrusoz. Would you provide me with some of that relatively large SIF graphs if possible? |
OK then give it a go @metincansiper |
Here is a relatively large one: GitHub does not let me add a .sif here. So I zipped it. |
Thank you! |
It has been a long time since I have implemented topology grouping feature in this repository.
One thing to consider here is that while generating the keys we need to make a sorting operation on a list whose size is the number of connections of a node. However, if we can assume that the number of connections of any node is upper bounded by the total number of the nodes in the graph then this approach must be more efficient than the other ones.
@ugurdogrusoz I can implement this in an availability of me and make a PR to the unstable branch if that would work for you.
The text was updated successfully, but these errors were encountered: