[CRITICAL BUG] Inconsistent scores of XCOMET between v2.2.4 and v2.2.5

[de9uch1]

@ricardorei
Hi, I'm a developer of mbrs, a library for MBR decoding.
I found a critical bug of XCOMET through our unittests.

🐛 Bug

The details of this bug and the cause were also reported in NL2G/xCOMET-lite#8

To Reproduce

Run this example code on unbabel-comet==2.2.4 and 2.2.5.
https://huggingface.co/Unbabel/XCOMET-XL

We observed that some examples varied the scores up to 10%.

Expected behaviour

The XCOMET scores should be consistent between v2.2.4 and v2.2.5.

Environment

OS: Linux
Packaging pip
Version 2.2.5 (and <=2.2.4)

Additional context

This is caused by this change ( 0ccf951 ).
Now (v2.2.5), different hyperparameters are passed from the version <=2.2.4 when initializeing the XCOMETMetric class.
Specifically, layer_transformation argument was not passed and set to "softmax" automatically before, but "sparsemax" is set now.

What makes things even more complicated is that, because save_hyperparameters() is called in the XCOMETMetric.__init__() here, after initializing LayerwiseAttention; thus, sparsemax, which was not actually passed before, but was saved in hparams.yaml.

This can be fixed in two ways

1. Change hparams.yaml: change layer_transformation to "softmax" and load the model by comet.load_from_checkpoint(model_path, reload_hparams=True). reload_hparams=True option must be set, because the hyperparameters saved in the checkpoint file are loaded instead of hparams.yaml, if not set.
2. Modify the checkpoint file: Modify the hyperparameters dict saved in this checkpoint. Probably, this method does not require the change of load_from_checkpoint() arguments.

As you can see, the solution is simple, but I'm not sure it would be the change you intended, so I created an issue instead of a pull request.

Thank you.

[ricardorei]

@de9uch1 Thanks for spotting this. I'll fix it asap.

I'll try to release a fix before Monday

PS: I really like mbrs, Congrats!

[ricardorei]

This probably also impacts CometKiwi models which like XCOMET might have sparsemax in the hparams but were trained with softmax instead.

[ricardorei]

Ok I just merged a fix that seems to be working.

I realised that while CometKiwi/XCOMET models were not loading sparsemax the wmt22-comet-da was. My fix was:

1. replace the layer_transformation: sparsemax with sparsemax_patch on the hparams.yaml of the effected models (XCOMET/CometKiwi models).
2. when loading a checkpoint I check for the package version. If its >= 2.2.4 and the layer_transformation from the hparams.yaml is sparsemax_patch I load the model using softmax.

This works with any model. Models like Unbabel/wmt22-comet-da will still load sparsemax and loading the models where I replaced sparsemax with sparsemax_patch will still load softmax on older COMET versions because its the default behaviour when the layer_transformation != sparsemax

Your solutions were also a possibility but reload_hparams=True would then be used on every model or I would have to create a hardcoded list of models that have to reload the hparams. Modifying a pytorch lightning checkpoint can result in some unexpected errors with pytorch lightning version and I would have to do it for all models impacted (that was my next option but I think this was easier).

[ricardorei]

From >=2.2.4, there will be a very small difference in terms of scores when using comet-score because of model.half(). This is very minimal and I think its worth it giving the speed gains.

e.g:
version 2.2.4
hyp1.txt Segment 0 score: 0.8413
hyp1.txt Segment 1 score: 0.9604
hyp1.txt score: 0.9009

version 2.2.6
hyp1.txt Segment 0 score: 0.8416
hyp1.txt Segment 1 score: 0.9607
hyp1.txt score: 0.9011

[de9uch1]

I've confirmed all the scores are correct with v2.2.6.
Thank you for your quick response!

[ricardorei]

No worries! Thanks for pointing this out. That was a major oversight on my part.

[avidale]

Hi @ricardorei !
Could you please somehow flag it in the code that with newer versions of Unbabel-comet, the checkpoints have to be updated?

I downloaded a COMET checkpoint half a year ago (under v2.2.4), and when I recently updated the codebase to 2.2.7, the COMET scores silently changed because my checkpoint (cached locally by Hugginface) still does not have the update that replaces layer_transformation: sparsemax with layer_transformation: sparsemax_patch, and I spent quite some time investigating the reasons of this change.

It would be great to have it noted somewhere in the readme that the fresh model files require fresh code (and vice versa).

Also, it might be a good idea to include the expected scores for a few examples in each model card, so that it would be easier to notice when the outputs diverge from these expectations.