SpeakerClustering: fix tensor dimennsions in forward() #5387

virajkarandikar · 2022-11-10T13:28:54Z

For some paramters in SpeakerClustering.forward(), tensor dimensions were not specified and this resulted in zero dimension tensor inputs. This causes error when inferencing exported model in Triton server.

What does this PR do ?

Avoids zero dimension input tensors. Also extracts values from the tensors which have single dim of size 1. This avoids scattered use of item() to extract value from tensor.

Collection: ASR Diarizer

Changelog

Add specific line by line info of high level changes in this PR.

Usage

Fix is needed to use exported clustering model in Triton

# Add a code snippet demonstrating how to use this

Before your PR is "Ready for review"

Pre checks:

Make sure you read and followed Contributor guidelines
Did you write any new necessary tests?
Did you add or update any necessary documentation?
Does the PR affect components that are optional to install? (Ex: Numba, Pynini, Apex etc)
Reviewer: Does the PR have correct import guards for all optional libraries?

PR Type:

Bugfix

If you haven't finished some of the above items you can still open "Draft" PR.

Who can review?

Anyone in the community is free to review the PR once the checks have passed.
Contributor guidelines contains specific people who can review PRs to various areas.

Additional Information

Related to # (issue)

For some paramters in SpeakerClustering.forward(), tensor dimensions were not specified. This results in error when inferencing exported model in Triton server. Signed-off-by: Viraj Karandikar <[email protected]>

virajkarandikar · 2022-11-10T13:58:36Z

@tango4j can you review this?

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

for more information, see https://pre-commit.ci

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

…karandikar/NeMo into vkarandikar_fix_clustering

for more information, see https://pre-commit.ci

tango4j · 2022-11-10T23:59:56Z

@virajkarandikar The change looks good. I have added unit-test for jit script exporting. However, this can only check problems on the python side, not for triton server cases.

nithinraok · 2022-11-11T00:30:21Z

tests/collections/asr/test_diar_utils.py

+    return emb_tensor, segm_tensor, multiscale_segment_counts, multiscale_weights, spk_timestamps
+
+
+@pytest.mark.skip("test is not intended")


why are we skipping this?

This function is sub-function for other testcases. This function was included in the test hogging up the time so skipped it.

This allows triton inference using human readable dict keys for parameter names instead of cryptic INPUT_x notation Signed-off-by: Viraj Karandikar <[email protected]>

virajkarandikar · 2022-11-11T06:07:53Z

@tango4j I have changed the forward to use Dict. This allows triton inference by using parameter names instead of INPUT_n notation. See https://github.com/triton-inference-server/server/blob/main/docs/user_guide/model_configuration.md#special-conventions-for-pytorch-backend for details.

Looks like the unit test needs to be updated. Will you do that please? I will try if I can.

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

for more information, see https://pre-commit.ci

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

…karandikar/NeMo into vkarandikar_fix_clustering

tango4j · 2022-11-12T02:59:36Z

@virajkarandikar The cluster inference function should have parameters for re-usability. so I made a wrapper function for the part you created (forward). I made nemo users to use forward_infer to have parameters in the function call. I think this will work out but please let me know if this does not compile

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

…karandikar/NeMo into vkarandikar_fix_clustering

for more information, see https://pre-commit.ci

virajkarandikar · 2022-11-12T09:43:42Z

Ok. It works fine.
Can you add test for forward() function also?

tango4j · 2022-11-12T22:10:15Z

@virajkarandikar Sure, I will add a test for forward().

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

…karandikar/NeMo into vkarandikar_fix_clustering

for more information, see https://pre-commit.ci

tango4j · 2022-11-14T20:34:02Z

@virajkarandikar I have added the test-case for .forward() (triton env inference). @nithinraok Please approve if there is no issues with this PR. Tutorial notebooks are also tested without errors.

* SpeakerClustering: fix tensor dimennsions in forward() For some paramters in SpeakerClustering.forward(), tensor dimensions were not specified. This results in error when inferencing exported model in Triton server. Signed-off-by: Viraj Karandikar <[email protected]> * adding tests for clustering Signed-off-by: Taejin Park <[email protected]> * removed unused functions Signed-off-by: Taejin Park <[email protected]> * removed unused functions 2 Signed-off-by: Taejin Park <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fixed cpu/gpu test decorator for unittests Signed-off-by: Taejin Park <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * clustering: use dict in forward This allows triton inference using human readable dict keys for parameter names instead of cryptic INPUT_x notation Signed-off-by: Viraj Karandikar <[email protected]> * created a wrapper for param_dicts for triton infer Signed-off-by: Taejin Park <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fixed docstrings Signed-off-by: Taejin Park <[email protected]> * fixed docstring Signed-off-by: Taejin Park <[email protected]> * fixed test_speaker_counting function in the test Signed-off-by: Taejin Park <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Added a test for Triton server call Signed-off-by: Taejin Park <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Signed-off-by: Viraj Karandikar <[email protected]> Signed-off-by: Taejin Park <[email protected]> Co-authored-by: Taejin Park <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Signed-off-by: Hainan Xu <[email protected]>

* SpeakerClustering: fix tensor dimennsions in forward() For some paramters in SpeakerClustering.forward(), tensor dimensions were not specified. This results in error when inferencing exported model in Triton server. Signed-off-by: Viraj Karandikar <[email protected]> * adding tests for clustering Signed-off-by: Taejin Park <[email protected]> * removed unused functions Signed-off-by: Taejin Park <[email protected]> * removed unused functions 2 Signed-off-by: Taejin Park <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fixed cpu/gpu test decorator for unittests Signed-off-by: Taejin Park <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * clustering: use dict in forward This allows triton inference using human readable dict keys for parameter names instead of cryptic INPUT_x notation Signed-off-by: Viraj Karandikar <[email protected]> * created a wrapper for param_dicts for triton infer Signed-off-by: Taejin Park <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fixed docstrings Signed-off-by: Taejin Park <[email protected]> * fixed docstring Signed-off-by: Taejin Park <[email protected]> * fixed test_speaker_counting function in the test Signed-off-by: Taejin Park <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Added a test for Triton server call Signed-off-by: Taejin Park <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Signed-off-by: Viraj Karandikar <[email protected]> Signed-off-by: Taejin Park <[email protected]> Co-authored-by: Taejin Park <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* SpeakerClustering: fix tensor dimennsions in forward() For some paramters in SpeakerClustering.forward(), tensor dimensions were not specified. This results in error when inferencing exported model in Triton server. Signed-off-by: Viraj Karandikar <[email protected]> * adding tests for clustering Signed-off-by: Taejin Park <[email protected]> * removed unused functions Signed-off-by: Taejin Park <[email protected]> * removed unused functions 2 Signed-off-by: Taejin Park <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fixed cpu/gpu test decorator for unittests Signed-off-by: Taejin Park <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * clustering: use dict in forward This allows triton inference using human readable dict keys for parameter names instead of cryptic INPUT_x notation Signed-off-by: Viraj Karandikar <[email protected]> * created a wrapper for param_dicts for triton infer Signed-off-by: Taejin Park <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * fixed docstrings Signed-off-by: Taejin Park <[email protected]> * fixed docstring Signed-off-by: Taejin Park <[email protected]> * fixed test_speaker_counting function in the test Signed-off-by: Taejin Park <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Added a test for Triton server call Signed-off-by: Taejin Park <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Signed-off-by: Viraj Karandikar <[email protected]> Signed-off-by: Taejin Park <[email protected]> Co-authored-by: Taejin Park <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Signed-off-by: andrusenkoau <[email protected]>

SpeakerClustering: fix tensor dimennsions in forward()

439d93f

For some paramters in SpeakerClustering.forward(), tensor dimensions were not specified. This results in error when inferencing exported model in Triton server. Signed-off-by: Viraj Karandikar <[email protected]>

virajkarandikar force-pushed the vkarandikar_fix_clustering branch from 4274f39 to 439d93f Compare November 10, 2022 13:29

tango4j added 4 commits November 10, 2022 13:05

adding tests for clustering

4b6a597

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

removed unused functions

b4a994f

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

removed unused functions 2

7f0f29d

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

Merge branch 'main' into vkarandikar_fix_clustering

d41d619

tango4j marked this pull request as ready for review November 10, 2022 21:15

[pre-commit.ci] auto fixes from pre-commit.com hooks

9072bdb

for more information, see https://pre-commit.ci

tango4j requested a review from nithinraok November 10, 2022 21:40

tango4j and others added 3 commits November 10, 2022 15:49

fixed cpu/gpu test decorator for unittests

5eef550

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

Merge branch 'vkarandikar_fix_clustering' of https://github.com/viraj…

5f74baf

…karandikar/NeMo into vkarandikar_fix_clustering

[pre-commit.ci] auto fixes from pre-commit.com hooks

ef652b4

for more information, see https://pre-commit.ci

Merge branch 'main' into vkarandikar_fix_clustering

d01a991

nithinraok reviewed Nov 11, 2022

View reviewed changes

clustering: use dict in forward

dafa2de

This allows triton inference using human readable dict keys for parameter names instead of cryptic INPUT_x notation Signed-off-by: Viraj Karandikar <[email protected]>

virajkarandikar force-pushed the vkarandikar_fix_clustering branch from 4dc1d85 to dafa2de Compare November 11, 2022 06:06

tango4j and others added 6 commits November 11, 2022 18:37

created a wrapper for param_dicts for triton infer

28471cf

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

[pre-commit.ci] auto fixes from pre-commit.com hooks

192d462

for more information, see https://pre-commit.ci

fixed docstrings

124fe07

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

fixed docstring

66205a4

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

Merge branch 'vkarandikar_fix_clustering' of https://github.com/viraj…

dbc0aa7

…karandikar/NeMo into vkarandikar_fix_clustering

Merge branch 'main' into vkarandikar_fix_clustering

5c866b5

tango4j and others added 3 commits November 11, 2022 21:05

fixed test_speaker_counting function in the test

b394897

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

Merge branch 'vkarandikar_fix_clustering' of https://github.com/viraj…

0577a1f

…karandikar/NeMo into vkarandikar_fix_clustering

[pre-commit.ci] auto fixes from pre-commit.com hooks

2703d9b

for more information, see https://pre-commit.ci

tango4j and others added 4 commits November 14, 2022 11:26

Added a test for Triton server call

bd82b1b

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

Merge branch 'vkarandikar_fix_clustering' of https://github.com/viraj…

bf7fc0b

…karandikar/NeMo into vkarandikar_fix_clustering

[pre-commit.ci] auto fixes from pre-commit.com hooks

b26b876

for more information, see https://pre-commit.ci

Merge branch 'main' into vkarandikar_fix_clustering

c6821ee

nithinraok approved these changes Nov 14, 2022

View reviewed changes

tango4j merged commit bd569bf into NVIDIA:main Nov 15, 2022

virajkarandikar deleted the vkarandikar_fix_clustering branch November 21, 2022 05:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SpeakerClustering: fix tensor dimennsions in forward() #5387

SpeakerClustering: fix tensor dimennsions in forward() #5387

virajkarandikar commented Nov 10, 2022

virajkarandikar commented Nov 10, 2022

tango4j commented Nov 10, 2022

nithinraok Nov 11, 2022

tango4j Nov 11, 2022

virajkarandikar commented Nov 11, 2022 •

edited

Loading

tango4j commented Nov 12, 2022

virajkarandikar commented Nov 12, 2022

tango4j commented Nov 12, 2022

tango4j commented Nov 14, 2022 •

edited

Loading

		return emb_tensor, segm_tensor, multiscale_segment_counts, multiscale_weights, spk_timestamps


		@pytest.mark.skip("test is not intended")

SpeakerClustering: fix tensor dimennsions in forward() #5387

SpeakerClustering: fix tensor dimennsions in forward() #5387

Conversation

virajkarandikar commented Nov 10, 2022

What does this PR do ?

Changelog

Usage

Before your PR is "Ready for review"

Who can review?

Additional Information

virajkarandikar commented Nov 10, 2022

tango4j commented Nov 10, 2022

nithinraok Nov 11, 2022

Choose a reason for hiding this comment

tango4j Nov 11, 2022

Choose a reason for hiding this comment

virajkarandikar commented Nov 11, 2022 • edited Loading

tango4j commented Nov 12, 2022

virajkarandikar commented Nov 12, 2022

tango4j commented Nov 12, 2022

tango4j commented Nov 14, 2022 • edited Loading

virajkarandikar commented Nov 11, 2022 •

edited

Loading

tango4j commented Nov 14, 2022 •

edited

Loading