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

Implement Pytorch Metrics #158

Open
bonham79 opened this issue Feb 6, 2024 · 6 comments · May be fixed by #284
Open

Implement Pytorch Metrics #158

bonham79 opened this issue Feb 6, 2024 · 6 comments · May be fixed by #284
Assignees
Labels
enhancement New feature or request

Comments

@bonham79
Copy link
Collaborator

bonham79 commented Feb 6, 2024

TorchMetrics support is pretty reliable nowadays and makes distributed training less annoying (no more World sizes, yay!). It also syncs well with Wandb logging and allows monitoring of training batch performance. Any complaints about me migrating validation logging to this?

@bonham79 bonham79 self-assigned this Feb 6, 2024
@kylebgorman
Copy link
Contributor

I don't know anything about it but it sounds like a positive.

@kylebgorman kylebgorman added the enhancement New feature or request label Feb 6, 2024
@bonham79
Copy link
Collaborator Author

bonham79 commented Feb 6, 2024

It's just a suite of metrics that torch sets up to manage multi-gpus under the hood. You just pass it your tensors during the training loop and it will store submetrics. Then when you need the actual metric you call and it does the calculation and memory collection under the hood. It saves you from desync issues if you have more than GPU for training. Also PTL supports it so it helps reduce boilerplate for other metrics.

@Adamits
Copy link
Collaborator

Adamits commented Feb 6, 2024

I think I tried to do this last year, and had some issues getting the features I actually wanted from it to track, so I gave up, but I think it was in beta or something then. I'd be happy if you got it working :)

@bonham79
Copy link
Collaborator Author

bonham79 commented Feb 6, 2024

I just got it running for BLEU scores on another project, so CER/WER should be stable by now.

@kylebgorman
Copy link
Contributor

kylebgorman commented Dec 1, 2024

I piloted this a bit. We can do our form of accuracy using torchmetrics.classification.MulticlassExactMatch, but they don't have an equivalent of our SER, just edit distance limited to strings. (So we'd have to convert back to strings, add all the necessary bookkeeping of letting the models see the indexes, and we wouldn't really be doing "SER" anymore either.) I filed a feature request but probably the best solution is to just use the same API they do to implement SER rather than submitting it to their library.

@bonham79
Copy link
Collaborator Author

bonham79 commented Dec 7, 2024

Yeah, they have a bias towards strings as final output, it's a bit annoying since you need to do metric calculations on CPU at terminus. It's somewhat painless to implement new metrics so long as the distributed training is managed properly. I support implementing it on our side and then once it's robust enough we can push to library if they ever allow more robust metric balancing.

kylebgorman added a commit to kylebgorman/yoyodyne that referenced this issue Dec 8, 2024
Closes CUNY-CL#158.

* Loss is computed as before, but streamlined somewhat.
* `torchmetrics`' implementation of exact match accuracy is lightly
  adapted. This does everything in tensor-land and should keep things on
  devices. My tests confirm that accuracy is EXACTLY what it was before.
* A `torchmetrics`-compatible implementation of symbol error rate (here
  defined as the edit distance divided by sum of target lengths) is
  inserted here. This is heavily documented and it is compatible with
  our existing implementation. The hot inner loop is still on CPU, but
  as mentioned in the documentation, this is probably the best option
  and I don't observe any obvious performance penalty when enabling
  this.
* We do away with the `evaluation` module altogether. Rather we treat
  the metrics objects as nullables living in the base class, a design
  adapted from UDTube.

The CLI interface is unimpacted, and my side-by-side shows the metrics
are exactly the same as before this change.
@kylebgorman kylebgorman linked a pull request Dec 8, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants