Skip to content
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

Open
metincansiper opened this issue Jun 2, 2022 · 8 comments
Open

Updating topological grouping algorithm #125

metincansiper opened this issue Jun 2, 2022 · 8 comments

Comments

@metincansiper
Copy link
Collaborator

metincansiper commented Jun 2, 2022

It has been a long time since I have implemented topology grouping feature in this repository.

  • I recognized that the algorithm I choose by that time is less efficient then I thought and also a few implementation mistakes I made makes it even less efficient
  • The other approach I considered but in a way ended up not implementing by that time is actually more efficient. The other approach I mentioned is basically is as below:
main()
  - maintain a hash table named as groupsTable
  - for each node:
    - Create a key by calling generateKey(node)
    - push the node to the groupsTable[key] (if groupsTable[key] is not existing initialize it with just this node)
  - Each key in the groupsTable represents a group so return the values of the table
 
generateKey(node)
  - maintain a string array 
  - for each edge of node
    -  Build a string that contains the other end of the edge, a divider ( _ ), an identifier representing the edge type, a divider ( _ ), a character representing the role of the other end in the edge (s for source, t for target, u if edge is undirected) 
    -  Push the string to the string array
  - Sort the string array
  - Join the elements of string array and return the result

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.

@ugurdogrusoz
Copy link
Contributor

@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 :)

@metincansiper
Copy link
Collaborator Author

metincansiper commented Jun 2, 2022

@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 :)

@ugurdogrusoz
Copy link
Contributor

What do you think @ozgunbabur?

@ozgunbabur
Copy link

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.

@metincansiper
Copy link
Collaborator Author

@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?

@ugurdogrusoz
Copy link
Contributor

OK then give it a go @metincansiper

@ozgunbabur
Copy link

Here is a relatively large one:
causative.zip

GitHub does not let me add a .sif here. So I zipped it.

@metincansiper
Copy link
Collaborator Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants