pyannote / pyannote-audio

Neural building blocks for speaker diarization: speech activity detection, speaker change detection, overlapped speech detection, speaker embedding
http://pyannote.github.io
MIT License
6.38k stars 784 forks source link

Remove pytorch_metric_learning dependency #1778

Closed pchampio closed 3 weeks ago

pchampio commented 3 weeks ago

pytorch-metric-learning has this numpy requirement that makes it hard to work with other package needing numpy > 2.0.

https://github.com/KevinMusgrave/pytorch-metric-learning/blob/master/setup.py#L42

Given the uses of pytorch-metric-learning in pyannote, which is only ArcFaceLoss, isn't it better to simply remove the pytorch-metric-learning deps and have 100 more line of code ?

Given the complexity of pytorch-metric-learning (lots of mixins), I came up with this version which I think is similar. Testing code:

torch.manual_seed(0)
num_classes = 2
embedding_size = 10
margin = 0.3
scale = 1
batch_size = 5
embeddings = torch.randn(batch_size, embedding_size)
labels = torch.randint(0, num_classes, (batch_size,))

af_own = ArcFaceLoss(num_classes, embedding_size, margin, scale)
loss = af_own.forward(embeddings, labels)
print(loss)

from pytorch_metric_learning.losses import ArcFaceLoss as afpytml
af = afpytml(
    num_classes, embedding_size, margin, scale
)
af.W = af_own.W
loss = af.forward(embeddings, labels)
print(loss)

I did not retrained a model to fully verify.

hbredin commented 3 weeks ago

Thanks @pchampio.

I'd rather go with a different option like, for instance, make dependencies used only for training purposes optional. One reason is that we are currently working on actually training new speaker embeddings within pyannote so making good use of pytorch-metric-learning.

pip install pyannote.audio
# does not install pytorch-metric-learning and other training only dependencies

pip install pyannote.audio[train]
# installs dependencies needed for training

It is very possible that this would require more changes than just editing setup.py though.

pchampio commented 3 weeks ago

Ok, thanks for the response.

It is very possible that this would require more changes than just editing setup.py though.

Indeed.

There is a 'hacky' way to have optional dependencies. Here is an example if you are interested. optional_deps.py It is a bit 'hacky' but it is working well, at least it has never failed for me.

pchampio commented 2 weeks ago

Fixed in https://github.com/KevinMusgrave/pytorch-metric-learning/issues/727