-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[Explainability] binary_classification
mode + link prediction example
#6083
[Explainability] binary_classification
mode + link prediction example
#6083
Conversation
Codecov Report
@@ Coverage Diff @@
## master #6083 +/- ##
=======================================
Coverage 84.48% 84.49%
=======================================
Files 371 371
Lines 20725 20738 +13
=======================================
+ Hits 17509 17522 +13
Misses 3216 3216
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@camillepradel Any reason to close this? We would still like to integrate an example of this :) |
you are right. I wanted to open a new PR but there is actually no good reason to not keep using this one. |
binary_classification
mode + link prediction example
So I updated the example, but since it was a link prediction task, I needed the explainability framework to support binary classification. I ended up adding a new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks a lot for the PR! I left some comments in the example file. Regarding the binary_classification
type, I also think we should find a simpler solution, e.g., by selecting the loss based on the models' output. I'll take a closer look.
Co-authored-by: Ramona Bendias <ramona.bendias@gmail.com>
for more information, see https://pre-commit.ci
I checked the code again. The main reason I see for the additional type is to check if the |
The distinction between You are right, we don't have to enforce |
I updated the code to allow More specifically, I don't know how to apply MSE loss directly on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, a few nit-picky comments. I think we need to drop log_probs
support in binary_classification
.
Co-authored-by: Matthias Fey <matthias.fey@tu-dortmund.de>
for more information, see https://pre-commit.ci
…lainer_binary_classification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates. Looks great!
Progress towards #5924
Done:
ModelMode.classification
intoModelMode.binary_classification
andModelMode.multiclass_classification
gnn_explainer_link_pred.py
training a link prediction model and getting the explanation for one output.