google / uis-rnn

This is the library for the Unbounded Interleaved-State Recurrent Neural Network (UIS-RNN) algorithm, corresponding to the paper Fully Supervised Speaker Diarization.

https://arxiv.org/abs/1810.04719

Apache License 2.0

1.56k stars 319 forks source link

corrected transition_bias estimation, fixes #55 #57

Closed DonkeyShot21 closed 5 years ago

DonkeyShot21 commented 5 years ago

This pull request fixes issue #55.

What is in this pull request:

new function uisrnn.utils.estimate_transition_bias that calculates transition_bias correctly
removed transition_bias estimation snippet from uisrnn.utils.resize_sequence. Now resize_sequence only retuns sub_sequences and seq_lengths
moved transition_bias update (for multiple calls to fit) from fit_concatenated to fit
added warning message if train_sequences is already concatenated and transition_bias has not been passed as argument, saying that the estimation can be incorrect
edited few test scripts to adapt to the new shape of the data returned from uisrnn.utils.resize_sequence

I also wanted to add _ to fit_concatenated to make it private so that the user is discouraged to call it, but I didn't because maybe it has some other dependency to some internal code of yours.

googlebot commented 5 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

DonkeyShot21 commented 5 years ago

Ok, sorry for the inconvenience.

googlebot commented 5 years ago

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

DonkeyShot21 commented 5 years ago

Ok, it took some time but I think we are good now. This is what I have added:

reverted all changes to docs
fixed the docstring
now the denominator takes double the smoothing of the numerator. This prevents both np.log(transition_bias), and np.log(1 - transition_bias) to go to -inf. Previous smoothing only worked with np.log(transition_bias)
simple test for the new function.

codecov-io commented 5 years ago

Codecov Report

Merging #57 into master will increase coverage by 0.18%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #57      +/-   ##
==========================================
+ Coverage   90.14%   90.32%   +0.18%     
==========================================
  Files           6        6              
  Lines         426      434       +8     
==========================================
+ Hits          384      392       +8     
  Misses         42       42

Impacted Files	Coverage Δ
uisrnn/uisrnn.py	`94.36% <100%> (+0.1%)`	:arrow_up:
uisrnn/utils.py	`81.74% <100%> (+0.59%)`	:arrow_up:

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 2e3b9a8...4da51a2. Read the comment docs.

codecov-io commented 5 years ago

Codecov Report

Merging #57 into master will increase coverage by 0.18%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #57      +/-   ##
==========================================
+ Coverage   90.14%   90.32%   +0.18%     
==========================================
  Files           6        6              
  Lines         426      434       +8     
==========================================
+ Hits          384      392       +8     
  Misses         42       42

Impacted Files	Coverage Δ
uisrnn/uisrnn.py	`94.36% <100%> (+0.1%)`	:arrow_up:
uisrnn/utils.py	`81.74% <100%> (+0.59%)`	:arrow_up:

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 2e3b9a8...4da51a2. Read the comment docs.