search

divelab / DIG

A library for graph deep learning research
https://diveintographs.readthedocs.io/
GNU General Public License v3.0
1.84k stars 281 forks source link

Symmetric edge masks in `GNNExplainer` and `GradCAM` for indirect graphs #144

Closed alirezadizaji closed 1 year ago

alirezadizaji commented 2 years ago

Hi, I think indirect graph datasets (like ba-2motifs) had better have symmetric edge masks because it doesn't mean to have two different probabilities for an indirect edge. If you agree about this, what do you think about handling it at the end of these two methods? thanks

alirezadizaji commented 2 years ago

If you agree please let me know to create a pull request for it

CM-BF commented 2 years ago

Thank you for your suggestion. My opinion is that it is the message passing nature to have directions, e.g., the message passing from the base graph to motifs and the message passing from motifs to base graph can be considered differently. But I also agree that applying the same probability for an indirect edge has its application scenarios. Therefore, I think adding an optional argument for this feature is a good idea.

Please let me know if there is any feedback. Thank you!

alirezadizaji commented 2 years ago

Thanks so I will create a pull request for this option

alirezadizaji commented 1 year ago

Sorry for my late response, I was a bit busy on another project but now I am working on this feature. I think it had better realize whether a graph is indirect or not by its edge_index, I mean if all edges have their reverse then we can conclude it is indirect, and This is better instead of considering option because this is something that we should understand from the graph input. what do you think? Or instead, it is better to define that optional boolean feature like this, whenever it is true, then explainer should realize whether it is a indirect graph and then make its returned weights symmetric.