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.

Sign up for GitHub

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

Jump to bottom

Update nmse clustering #2445

Merged
merged 7 commits into from
Jul 9, 2021
Merged

Update nmse clustering #2445

merged 7 commits into from
Jul 9, 2021

Conversation

tango4j
Copy link
Collaborator

@tango4j tango4j commented Jul 6, 2021

  • Speed up on eigen decomposition by using GPU.
  • Speed up on NME analysis by subsampling.
  • Fixed the diarization evaluation parameter (no score collar: from 0.25 s to 0.5 s)

@lgtm-com
Copy link

lgtm-com bot commented Jul 6, 2021

This pull request introduces 3 alerts and fixes 2 when merging 7583dd9 into c5dbf45 - view on LGTM.com

new alerts:

  • 3 for Unused local variable

fixed alerts:

  • 2 for Except block handles 'BaseException'

nithinraok
nithinraok requested changes Jul 7, 2021
View reviewed changes
Copy link
Collaborator

@nithinraok nithinraok left a comment

Choose a reason for hiding this comment

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

overall looks good, minor fixes and changes are required.

nemo/collections/asr/parts/utils/speaker_utils.py Outdated Show resolved Hide resolved
nemo/collections/asr/parts/utils/speaker_utils.py Show resolved Hide resolved
nemo/collections/asr/parts/utils/nmse_clustering.py Show resolved Hide resolved
nemo/collections/asr/parts/utils/nmse_clustering.py Show resolved Hide resolved
nemo/collections/asr/parts/utils/nmse_clustering.py Show resolved Hide resolved
nemo/collections/asr/parts/utils/nmse_clustering.py Outdated Show resolved Hide resolved
nemo/collections/asr/parts/utils/nmse_clustering.py Outdated Show resolved Hide resolved
nemo/collections/asr/parts/utils/nmse_clustering.py Outdated Show resolved Hide resolved
nemo/collections/asr/parts/utils/nmse_clustering.py Outdated Show resolved Hide resolved
nemo/collections/asr/parts/utils/nmse_clustering.py Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Jul 7, 2021

This pull request introduces 2 alerts and fixes 2 when merging 363107b into c5dbf45 - view on LGTM.com

new alerts:

  • 2 for Unused local variable

fixed alerts:

  • 2 for Except block handles 'BaseException'

nithinraok
nithinraok requested changes Jul 8, 2021
View reviewed changes
Copy link
Collaborator

@nithinraok nithinraok left a comment

Choose a reason for hiding this comment

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

so now pyamg dependency is not required right for Eigen solver? we can remove that from requirements.

nemo/collections/asr/parts/utils/nmse_clustering.py Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Jul 8, 2021

This pull request introduces 3 alerts and fixes 3 when merging 80f5a9b into e5bde15 - view on LGTM.com

new alerts:

  • 2 for Unused local variable
  • 1 for Testing equality to None

fixed alerts:

  • 2 for Except block handles 'BaseException'
  • 1 for Variable defined multiple times

ericharper
ericharper approved these changes Jul 9, 2021
View reviewed changes
Copy link
Collaborator

@ericharper ericharper left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@nithinraok nithinraok merged commit 17133d0 into main Jul 9, 2021
@nithinraok nithinraok deleted the update_nmse_clustering branch July 9, 2021 00:53
pasandi20 pushed a commit to pasandi20/NeMo that referenced this pull request Jul 13, 2021
@tango4j @nithinraok
Update nmse clustering (NVIDIA#2445)
5d6092c 
* Updating refined NMSE clustering code to run on GPU.

Signed-off-by: Taejin Park <[email protected]>

* updated evaluation accuracy decimals and fixed bugs in clustering

Signed-off-by: Taejin Park <[email protected]>

* Style fix.

Signed-off-by: Taejin Park <[email protected]>

* Doc strings added and few revisions. Reflected comments by Nithin.

Signed-off-by: Taejin Park <[email protected]>

* Refelected second round of comments, simplified code and removed pyamg from req.

Signed-off-by: Taejin Park <[email protected]>

* Style fix again

Signed-off-by: Taejin Park <[email protected]>

Co-authored-by: Nithin Rao <[email protected]>
Signed-off-by: Ghasem Pasandi <[email protected]>
fayejf pushed a commit that referenced this pull request Jul 16, 2021
@tango4j @nithinraok @fayejf
Update nmse clustering (#2445)
81e3516 
* Updating refined NMSE clustering code to run on GPU.

Signed-off-by: Taejin Park <[email protected]>

* updated evaluation accuracy decimals and fixed bugs in clustering

Signed-off-by: Taejin Park <[email protected]>

* Style fix.

Signed-off-by: Taejin Park <[email protected]>

* Doc strings added and few revisions. Reflected comments by Nithin.

Signed-off-by: Taejin Park <[email protected]>

* Refelected second round of comments, simplified code and removed pyamg from req.

Signed-off-by: Taejin Park <[email protected]>

* Style fix again

Signed-off-by: Taejin Park <[email protected]>

Co-authored-by: Nithin Rao <[email protected]>
titu1994 pushed a commit to titu1994/NeMo that referenced this pull request Jul 20, 2021
@tango4j @nithinraok @titu1994
Update nmse clustering (NVIDIA#2445)
67432a8 
* Updating refined NMSE clustering code to run on GPU.

Signed-off-by: Taejin Park <[email protected]>

* updated evaluation accuracy decimals and fixed bugs in clustering

Signed-off-by: Taejin Park <[email protected]>

* Style fix.

Signed-off-by: Taejin Park <[email protected]>

* Doc strings added and few revisions. Reflected comments by Nithin.

Signed-off-by: Taejin Park <[email protected]>

* Refelected second round of comments, simplified code and removed pyamg from req.

Signed-off-by: Taejin Park <[email protected]>

* Style fix again

Signed-off-by: Taejin Park <[email protected]>

Co-authored-by: Nithin Rao <[email protected]>
paarthneekhara pushed a commit to paarthneekhara/NeMo that referenced this pull request Sep 17, 2021
@tango4j @nithinraok @paarthneekhara
Update nmse clustering (NVIDIA#2445)
a9706a5 
* Updating refined NMSE clustering code to run on GPU.

Signed-off-by: Taejin Park <[email protected]>

* updated evaluation accuracy decimals and fixed bugs in clustering

Signed-off-by: Taejin Park <[email protected]>

* Style fix.

Signed-off-by: Taejin Park <[email protected]>

* Doc strings added and few revisions. Reflected comments by Nithin.

Signed-off-by: Taejin Park <[email protected]>

* Refelected second round of comments, simplified code and removed pyamg from req.

Signed-off-by: Taejin Park <[email protected]>

* Style fix again

Signed-off-by: Taejin Park <[email protected]>

Co-authored-by: Nithin Rao <[email protected]>
Signed-off-by: Paarth Neekhara <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nithinraok nithinraok nithinraok approved these changes

@ericharper ericharper ericharper approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tango4j @ericharper @nithinraok