eole-nlp / eole

Open language modeling toolkit based on PyTorch
https://eole-nlp.github.io/eole
MIT License
62 stars 12 forks source link

Add comet scorer #118

Open anthdr opened 1 month ago

anthdr commented 1 month ago

Added comet scorers for validation in NMT training. I left progress_bar=True, num_workers=0, batch_size=64 and gpu=0 for cpu inference, should it be like that by default and should it be modifiable via config ? I did not modify tests as it would greatly extend the tests duration (downloading comet models and inferencing on cpu). I also took the liberty to lightly modify wmt17 recipe/doc to make it more beginner friendly.

Fix #34

vince62s commented 1 month ago

Thanks for your PR! I think it would be better to use a single block of code for all comet-like scorer and add a setting for the model path at start-up.

vince62s commented 1 month ago

In a real world, COMET requires a GPU so it would be great to unload the model being trained or/and specify another gpu for comet. What do you think ?

anthdr commented 1 month ago

Continuing the discussion here: There are multiple way for doing this, off-loading current model to run comet scoring on gpu as @vince62s suggested, we could run it parallel of the training (subprocessing a comet cpu scoring), dedicating a second gpu to score. That's why I first implemented it on cpu during eval (to not mess with vram) and have it working transparently for a naiver user. What would you recommend ?

vince62s commented 1 month ago

I think the easiest for 1) user convenience 2) speed is to unload the model being trained, load the comet model on the same gpu, and relaod the trianed model after scoring. Scoring on cpu will be way too slow and dedicating a second gpu will be a nightmare to handle from the user standpoint