eneserdo
commented
4 days ago Thanks for the effort. Imho, you could have handled the "refining" on directly preds tensor using something like this:
preds_topk = torch.argsort(preds, dim=-1, descending=True)[:, :top_k]
preds_top1 = preds_topk[:, 0]
preds=torch.where((target.view(-1, 1) == preds_topk).sum(dim=-1).bool(), target, preds_top1)Which is more compact way of doing the same job. (Cloning and reshaping are omitted here)
Also, these changes will break the current tests for all top_k related classes/functions e.g. for recall, accuracy, f1, so on so forth. I think it is important to re-write these tests. Additionally, maybe for the topk accuracy you can take the scikit learn's top_k_accuracy_score as a reference.
rittik9
codecov[bot]
What does this PR do?
Fixes #1653
Before submitting
- [x] Was this **discussed/agreed** via a Github issue? (no need for typos and docs improvements) - [x] Did you read the [contributor guideline](https://github.com/Lightning-AI/torchmetrics/blob/master/.github/CONTRIBUTING.md), Pull Request section? - [x] Did you make sure to **update the docs**? - [x] Did you write any new **necessary tests**?PR review
Anyone in the community is free to review the PR once the tests have passed. If we didn't discuss your PR in Github issues there's a high chance it will not be merged.Did you have fun?
Make sure you had fun coding 🙃
📚 Documentation preview 📚: https://torchmetrics--2839.org.readthedocs.build/en/2839/