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

Using TochMetrics for code simplification #4283

Closed
Borda opened this issue Mar 17, 2022 · 2 comments · Fixed by #4287
Closed

Using TochMetrics for code simplification #4283

Borda opened this issue Mar 17, 2022 · 2 comments · Fixed by #4287

Comments

@Borda
Copy link
Contributor

Borda commented Mar 17, 2022

🛠 Proposed Refactor

We have been developing TochMetrics to be a general-purpose metric as well some domain-specific use-cases. In many cases, we have an exact mapping to scikit-learn with verification/testing to the reference metric for its correctness. The TM includes functional as well as nn.Module versions and for the most/standard metrics, the only dependency is Pytorch (for the domain-specific metrics you need to install related extras).

Suggest a potential alternative/fix

WIth using TM metrics you may rely on the widely tested correctness (testing against good standard scikit-learn in multiple OS environments and all PyTorch versions above v1.4) and later you can use nn.Module to leverage update and compute.

Overall if you are fine with it we are happy to draft a PR with a suggested change to verify in place the impact.
If you have any questions, happy to follow up with me or @tchaton

What I have quickly checked, al this torch_geometric.utils.metric can be replaced by TM

@Borda Borda added the refactor label Mar 17, 2022
@rusty1s
Copy link
Member

rusty1s commented Mar 17, 2022

Thanks for this great issue. We are already using torchmetrics in all our PL examples, and so I am happy to make the switch to it completely. torch_geometric.utils.metric is somewhat old and not well maintained. In fact, as far as I can see, only the intersection_and_union metric is actually used inside PyG and examples/.

As such, we could:

  • remove torch_geometric.utils.metric completely
  • Update the examples to make use of torchmetrics

Let me know what you think.

@Borda
Copy link
Contributor Author

Borda commented Mar 17, 2022

cool, thank you @rusty1s! I have started a draft PR as I may have a few additional questions to deliver the best...
so let's move the additional/implementation discussion to the open #4287

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

Successfully merging a pull request may close this issue.

2 participants