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

[WIP] Multilabel Detection #891

Merged
merged 53 commits into from Jul 1, 2022
Merged

Conversation

hadware
Copy link
Contributor

@hadware hadware commented Feb 14, 2022

This is a new PR for the VTC feature, this time based on a cleaner implem. I'm making a new PR as to keep the former branch "clean" (and prevent any mishaps).

What is done:

  • renaming the SpeakerTracking task into a MultilabelDetection task
  • added MultilabelPipeline
  • update MultilabelFscore.report()
  • tested the new preprocessor

What's to be done:

  • re-test the new implem on our clinical data (as well as the child data from @MarvinLvn )
  • maybe a couple of unit tests (especially for the preprocessor)
  • maybe make the aggregated "multilabel" fscore duration-based instead of file-based

@codecov
Copy link

codecov bot commented Feb 16, 2022

Codecov Report

Merging #891 (158e809) into develop (26cddd9) will decrease coverage by 1.09%.
The diff coverage is 6.01%.

❗ Current head 158e809 differs from pull request most recent head 4b4f249. Consider uploading reports for the commit 4b4f249 to get more accurate results

@@             Coverage Diff             @@
##           develop     #891      +/-   ##
===========================================
- Coverage    35.32%   34.23%   -1.10%     
===========================================
  Files           58       59       +1     
  Lines         3459     3584     +125     
===========================================
+ Hits          1222     1227       +5     
- Misses        2237     2357     +120     
Impacted Files Coverage Δ
pyannote/audio/pipelines/__init__.py 0.00% <0.00%> (ø)
pyannote/audio/pipelines/multilabel.py 0.00% <0.00%> (ø)
pyannote/audio/utils/metric.py 0.00% <0.00%> (ø)
pyannote/audio/utils/preprocessors.py 0.00% <0.00%> (ø)
pyannote/audio/tasks/segmentation/multilabel.py 90.90% <70.00%> (ø)
pyannote/audio/tasks/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26cddd9...4b4f249. Read the comment docs.

@hadware
Copy link
Contributor Author

hadware commented Mar 8, 2022

Alright, I've tested it again using the clinical data, the scores are analogous to what I got in the table in #694 . Marianne is going to test it on babytrain data, but she's currently on holiday (and for two weeks). If that's ok with you, i'd like to proceed to the part where you make sure that the code is mergeable, as I think that this should be good enough for a merge.

@hbredin
Copy link
Member

hbredin commented Mar 15, 2022

Seems to go in the right direction.
I will make a pass and update a few things myself.

In the meantime, could you add a notebook showing how to use this new objects to train a male/female/speech multi-label pipeline on top of Debug.SpeakerDiarization.Debug protocol available in tests/data/database.yml.

Basically, this would illustrate

  • the use of a preprocessor to convert Mxxxx labels to male and Fxxxx labels to female
  • the use of a VoiceTypeClassifierPreprocessor preprocessor to add the union of male and female as speech
  • a way to chain them
  • the new MultilabelDetection task
  • the new MultilabelDetectionpipeline

* rename MultilabelDetectionPipeline to MultilabelDetection
* rename MultilabelFMeasure to MacroAverageFMeasure and move it to pyannote.audio.utils.metric
* rename VoiceTypeClassifierPreprocessor to DeriveMetaLabels and move it to pyannote.audio.utils.preprocessors
* make segmentation model mandatory and remove default parameters
* add support for pipeline hook
* rename chunk_labels to ordered_labels
Copy link
Member

@hbredin hbredin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spent some time looking at the code. Looks good!
I do have a few questions and would love an update on the notebook :) Does it run as it is right now (refering to the link you shared earlier)?

pyannote/audio/pipelines/multilabel_detection.py Outdated Show resolved Hide resolved
for label in self._classes
}
)
# TODO: would it make sense to share min_duration_{on|off} between classes?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this particular task that may lead to a huge hyperparameter search space, we could also force onset == offset (once again through a flag use_hysteresis).

for label in self._classes
}
)
# TODO: would it make sense to share min_duration_{on|off} between classes?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But, let's do that in a separate PR.

num_workers: int = None,
pin_memory: bool = False,
augmentation: BaseWaveformTransform = None,
metric: Union[Metric, Sequence[Metric], Dict[str, Metric]] = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't thought a lot about the validation metric for this type of task.
It relies on AUROC right? But what does it mean for multilabel classification?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does rely on AUROC I think. I don't have an answer to this. I just used the metric provided by the class without thinking a second about what I was doing. 🙃

pyannote/audio/tasks/segmentation/multilabel_detection.py Outdated Show resolved Hide resolved
@hbredin
Copy link
Member

hbredin commented Jun 16, 2022

FYI - I just released pyannote.pipeline 2.3 with support for ParamDict.

@hadware
Copy link
Contributor Author

hadware commented Jun 20, 2022

All right

https://colab.research.google.com/drive/1JX3x4t_QTHMr8hzjw3Em8eQBQnQ3UBgk#scrollTo=U9lwdYcTHyp_ is working fine as an example (with your input on using a subset of AMI). The inferred annotations look pretty good, and the tuning step amounts to a noticeable gain of bout 5% of IER.

If you think this is good enough, I can then re-add the comments and tutorial text from the original notebook (with a couple of tweaks) and we can call it a day. I'll also merge the latest commits from "upstream".

@hadware
Copy link
Contributor Author

hadware commented Jun 20, 2022

I'll also take care of the comments you submitted in #891 (review)

@hbredin
Copy link
Member

hbredin commented Jun 20, 2022

All right

https://colab.research.google.com/drive/1JX3x4t_QTHMr8hzjw3Em8eQBQnQ3UBgk#scrollTo=U9lwdYcTHyp_ is working fine as an example (with your input on using a subset of AMI). The inferred annotations look pretty good, and the tuning step amounts to a noticeable gain of bout 5% of IER.

Awesome! I was able to run the whole thing successfully in 10min or so.
That is a great demo!

If you think this is good enough, I can then re-add the comments and tutorial text from the original notebook (with a couple of tweaks) and we can call it a day. I'll also merge the latest commits from "upstream".

Yes, please!

@hadware
Copy link
Contributor Author

hadware commented Jun 20, 2022

I re-added the text to the colab notebook, and added, implemented and tested the share_min_duration option

@hbredin
Copy link
Member

hbredin commented Jun 30, 2022

Hey @hadware, would you mind adding the notebook to this PR?

This should go in the tutorials directory with a link added in the README "Documentation / Miscellaneous" section.

@hadware
Copy link
Contributor Author

hadware commented Jun 30, 2022

Done.

@hadware
Copy link
Contributor Author

hadware commented Jun 30, 2022

Is there anything more needed to merge this?

@hbredin hbredin merged commit 1dac64a into pyannote:develop Jul 1, 2022
@hbredin
Copy link
Member

hbredin commented Jul 1, 2022

Hurrah! Looking forward to lots of new applications!

I removed the notebook because it felt like it was not clean enough.
Anyway, all notebooks should probably be updated to run on Google Colab with the new AMI Mini.
I'll open a dedicated issue.

Thanks again @hadware for the hard work and sorry for being such a pinailleur...

@hadware
Copy link
Contributor Author

hadware commented Jul 1, 2022

The pinailling is part of the fun 🥵

I'd like to pretrain a couple of models to host then on hugginface along with the others. What kind of classes/train datasets would you suggest? I was thinking about MALE/FEMALE using AMI/VoxCeleb, but that's it. Any other ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants