Add `fn.experimental.audio_resample` GPU #3911

jantonguirao · 2022-05-17T11:46:48Z

Category:

New feature

Description:

It adds a new operator fn.experimental.audio_resample for the GPU backend.
The operator builds on top of the existing CPU operator counterpart
The implementation relies on the GPU signal resampling kernel.

Additional information:

Affected modules and functionalities:

New Op

Key points relevant for the review:

Test correctness?

Checklist

Tests

Documentation

DALI team only

Requirements

Implements new requirements
Affects existing requirements
N/A

REQ IDs: ARES.[01-08]

JIRA TASK: DALI-1445

jantonguirao · 2022-05-17T11:49:31Z

dali/test/python/test_operator_audio_resample.py

@@ -34,64 +34,76 @@
 rates = [ 16000, 22050, 12347 ]
 lengths = [ 10000, 54321, 12345 ]

-def create_test_files():


Note: nose was picking this as a test

Making it "private" (with leading underscore) would help, too.

Signed-off-by: Michał Zientkiewicz <[email protected]>

Signed-off-by: Joaquin Anton <[email protected]>

Signed-off-by: Michał Zientkiewicz <[email protected]>

Signed-off-by: Joaquin Anton <[email protected]>

mzient · 2022-05-18T07:42:36Z

dali/operators/audio/resample.h

@@ -144,7 +148,8 @@ class ResampleBase : public Operator<Backend> {
  ArgValue<float> scale_{"scale", spec_};
  ArgValue<int64_t> out_length_{"out_length", spec_};

-  std::vector<double> scales_;
+  using Args = kernels::signal::resampling::Args;
+  SmallVector<Args, 128> args_;


Plain vector will do just fine. Large SmallVectors (however weird that sounds) are better suited for temporary local buffers, where the difference between (frequent) stack allocation (free) and heap allocation (thousands of cycles) is of essence. Here, the vector will be reallocated a few times per operator lifetime at worst (typically it will be allocated just once) - and chances are, we'll still run over 128.

Suggested change

SmallVector<Args, 128> args_;

std::vector<Args> args_;

Signed-off-by: Joaquin Anton <[email protected]>

…u_op Signed-off-by: Joaquin Anton <[email protected]>

jantonguirao · 2022-05-18T13:55:13Z

dali/kernels/signal/resampling_gpu.cu

@@ -77,13 +77,12 @@ void ResamplerGPU<Out, In>::Run(KernelContext &context, const OutListGPU<Out> &o
    desc.out = out_sample.data;
    desc.window = window_gpu_;
    const auto &in_sh = in_sample.shape;
-    const auto &out_sh = out_sample.shape;


removed this because it was seen as not-used in release builds (therefore a warning)

dali-automaton · 2022-05-19T07:20:24Z

CI MESSAGE: [4879759]: BUILD STARTED

dali-automaton · 2022-05-19T07:34:23Z

CI MESSAGE: [4879843]: BUILD STARTED

dali-automaton · 2022-05-19T08:21:24Z

CI MESSAGE: [4879843]: BUILD FAILED

dali-automaton · 2022-05-19T12:40:09Z

CI MESSAGE: [4881910]: BUILD STARTED

dali-automaton · 2022-05-19T15:18:01Z

CI MESSAGE: [4881910]: BUILD PASSED

mzient · 2022-05-20T07:57:38Z

dali/test/python/test_torch_pipeline_rnnt.py

@@ -1,4 +1,4 @@
-# Copyright (c) 2020, NVIDIA CORPORATION. All rights reserved.
+# Copyright (c) 2020-2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved.


Shouldn't this be a separate PR?

mzient · 2022-05-20T08:04:11Z

dali/test/python/test_operator_audio_resample.py

-        assert np.allclose(out.at(i), ref, 1e-6, eps)
-
+        print("Diff: ", out_arr.astype(np.float) - ref)
+        assert False


Nitpick: the original code deliberately repeated the check here, so that the error in nosetests would appear as more than False in non-verbose runs.

mzient · 2022-05-20T08:08:52Z

dali/operators/audio/resample.h

+        args_[s].in_rate = 1.0;
+        args_[s].out_rate = in_length ? 1.0 * out_length / in_length : 0.0;


Nitpick:
Now that we have args, this would increase precision - we use the inverse scale in the kernel (in_rate/out_rate), so performing the other division here and reciprocal there will decrease (albeit very slightly) the precision.

Suggested change

args_[s].in_rate = 1.0;

args_[s].out_rate = in_length ? 1.0 * out_length / in_length : 0.0;

args_[s].in_rate = in_length ? in_length : 1; // avoid division by 0

args_[s].out_rate = out_length ? out_length : 1; // avoid division by 0

Signed-off-by: Joaquin Anton <[email protected]>

mzient · 2022-05-20T08:26:28Z

dali/operators/audio/resample.h

@@ -125,7 +128,8 @@ class ResampleBase : public Operator<Backend> {
          DALI_FAIL(make_string("Cannot produce a non-empty signal from an empty input.\n"
            "Error at sample ", s));
        }
-        scales_[s] = in_length ? 1.0 * out_length / in_length : 0.0;
+        args_[s].in_rate  = in_length  ? in_length  : 1;  // avoid division by 0
+        args_[s].out_rate = out_length ? out_length : 1; // avoid division by 0


Suggested change

args_[s].out_rate = out_length ? out_length : 1; // avoid division by 0

args_[s].out_rate = out_length ? out_length : 1; // avoid division by 0

the linter will complain

Signed-off-by: Joaquin Anton <[email protected]>

awolant · 2022-05-20T11:30:10Z

dali/test/python/test_operator_audio_resample.py

@@ -34,64 +34,73 @@
 rates = [ 16000, 22050, 12347 ]
 lengths = [ 10000, 54321, 12345 ]

-def create_test_files():
+def create_files():


I think this would be another way, more verbose probably:

Suggested change

def create_files():

from nose.tools import nottest

@nottest

def create_files():

jantonguirao · 2022-05-20T12:22:01Z

!build

dali-automaton · 2022-05-20T12:25:37Z

CI MESSAGE: [4891719]: BUILD STARTED

dali-automaton · 2022-05-20T13:28:02Z

CI MESSAGE: [4891719]: BUILD PASSED

Signed-off-by: Joaquin Anton <[email protected]>

jantonguirao changed the title ~~Audio resampling gpu op~~ Add fn.experimental.audio_resample GPU May 17, 2022

jantonguirao marked this pull request as draft May 17, 2022 11:47

jantonguirao commented May 17, 2022

View reviewed changes

mzient and others added 15 commits May 17, 2022 15:56

Initial effort.

0778f61

Signed-off-by: Michał Zientkiewicz <[email protected]>

Add signal resampling GPU kernel

740bd14

Signed-off-by: Joaquin Anton <[email protected]>

Remove downmixing

5b410ed

Signed-off-by: Joaquin Anton <[email protected]>

Code review fixes

6065354

Signed-off-by: Joaquin Anton <[email protected]>

Add benchmark

7c0162d

Signed-off-by: Joaquin Anton <[email protected]>

Avoid precision issue & add shared memory usage

7a39c95

Signed-off-by: Joaquin Anton <[email protected]>

Move double in_block_f calculation inside the loop

3ef0f58

Signed-off-by: Joaquin Anton <[email protected]>

Fix benchmark

a66a763

Signed-off-by: Joaquin Anton <[email protected]>

Update benchmark

44354b2

Signed-off-by: Joaquin Anton <[email protected]>

ROI & input conversion to float & limit tmp shared mem

e4eb226

Signed-off-by: Joaquin Anton <[email protected]>

Add comments

13ff3db

Signed-off-by: Joaquin Anton <[email protected]>

Use floorf and ceilf in CUDA code

3467768

Signed-off-by: Joaquin Anton <[email protected]>

Improve tests & fix bugs

b10d860

Signed-off-by: Joaquin Anton <[email protected]>

Move resampling GPU to cu file & add sync to Initialize

19e58e8

Signed-off-by: Joaquin Anton <[email protected]>

Add audio_resample GPU operator

bd8212a

Signed-off-by: Joaquin Anton <[email protected]>

jantonguirao force-pushed the audio_resampling_gpu_op branch from aeb5f2d to bd8212a Compare May 17, 2022 13:56

mzient and others added 11 commits May 17, 2022 16:28

Initial effort.

bf32abc

Signed-off-by: Michał Zientkiewicz <[email protected]>

Add signal resampling GPU kernel

a983fdd

Signed-off-by: Joaquin Anton <[email protected]>

Remove downmixing

9a403ea

Signed-off-by: Joaquin Anton <[email protected]>

Code review fixes

c6374a4

Signed-off-by: Joaquin Anton <[email protected]>

Add benchmark

d88e78b

Signed-off-by: Joaquin Anton <[email protected]>

Avoid precision issue & add shared memory usage

074bc13

Signed-off-by: Joaquin Anton <[email protected]>

Move double in_block_f calculation inside the loop

5248048

Signed-off-by: Joaquin Anton <[email protected]>

Fix benchmark

3f74625

Signed-off-by: Joaquin Anton <[email protected]>

Update benchmark

177fced

Signed-off-by: Joaquin Anton <[email protected]>

ROI & input conversion to float & limit tmp shared mem

744f570

Signed-off-by: Joaquin Anton <[email protected]>

Add comments

12e177c

Signed-off-by: Joaquin Anton <[email protected]>

mzient reviewed May 18, 2022

View reviewed changes

jantonguirao added 2 commits May 18, 2022 13:44

Rebase

c5cec3b

Signed-off-by: Joaquin Anton <[email protected]>

Code review fixes

7a0cc7a

Signed-off-by: Joaquin Anton <[email protected]>

jantonguirao force-pushed the audio_resampling_gpu_op branch from fc49aae to 7a0cc7a Compare May 18, 2022 11:48

Test full GPU pipe

934c17e

Signed-off-by: Joaquin Anton <[email protected]>

jantonguirao force-pushed the audio_resampling_gpu_op branch from 4e08539 to 9409cc9 Compare May 18, 2022 13:54

Merge remote-tracking branch 'upstream/main' into audio_resampling_gp…

d76cd88

…u_op Signed-off-by: Joaquin Anton <[email protected]>

jantonguirao force-pushed the audio_resampling_gpu_op branch from 9409cc9 to d76cd88 Compare May 18, 2022 13:54

jantonguirao commented May 18, 2022

View reviewed changes

mzient reviewed May 20, 2022

View reviewed changes

jantonguirao force-pushed the audio_resampling_gpu_op branch from 70b6342 to a5782b4 Compare May 20, 2022 08:02

mzient reviewed May 20, 2022

View reviewed changes

mzient approved these changes May 20, 2022

View reviewed changes

Code review fixes

23525bf

Signed-off-by: Joaquin Anton <[email protected]>

jantonguirao force-pushed the audio_resampling_gpu_op branch from 4d9d2ae to 23525bf Compare May 20, 2022 08:16

mzient reviewed May 20, 2022

View reviewed changes

Code review fixes

b2c2c57

Signed-off-by: Joaquin Anton <[email protected]>

awolant approved these changes May 20, 2022

View reviewed changes

jantonguirao merged commit 9918cb5 into NVIDIA:main May 23, 2022

cyyever pushed a commit to cyyever/DALI that referenced this pull request Jun 7, 2022

Add fn.experimental.audio_resample GPU (NVIDIA#3911)

26ff9df

Signed-off-by: Joaquin Anton <[email protected]>

JanuszL mentioned this pull request Jan 11, 2023

DALI 2022 roadmap #3774

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `fn.experimental.audio_resample` GPU #3911

Add `fn.experimental.audio_resample` GPU #3911

jantonguirao commented May 17, 2022

jantonguirao May 17, 2022

mzient May 18, 2022

mzient May 18, 2022

jantonguirao May 18, 2022

dali-automaton commented May 19, 2022

dali-automaton commented May 19, 2022

dali-automaton commented May 19, 2022

dali-automaton commented May 19, 2022

dali-automaton commented May 19, 2022

mzient May 20, 2022

mzient May 20, 2022

mzient May 20, 2022 •

edited

Loading

mzient May 20, 2022

awolant May 20, 2022

jantonguirao commented May 20, 2022

dali-automaton commented May 20, 2022

dali-automaton commented May 20, 2022

		@@ -1,4 +1,4 @@
		# Copyright (c) 2020, NVIDIA CORPORATION. All rights reserved.
		# Copyright (c) 2020-2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved.

		args_[s].in_rate = 1.0;
		args_[s].out_rate = in_length ? 1.0 * out_length / in_length : 0.0;

	args_[s].out_rate = out_length ? out_length : 1; // avoid division by 0
	args_[s].out_rate = out_length ? out_length : 1; // avoid division by 0

Add fn.experimental.audio_resample GPU #3911

Add fn.experimental.audio_resample GPU #3911

Conversation

jantonguirao commented May 17, 2022

Category:

Description:

Additional information:

Affected modules and functionalities:

Key points relevant for the review:

Checklist

Tests

Documentation

DALI team only

Requirements

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dali-automaton commented May 19, 2022

dali-automaton commented May 19, 2022

dali-automaton commented May 19, 2022

dali-automaton commented May 19, 2022

dali-automaton commented May 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mzient May 20, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jantonguirao commented May 20, 2022

dali-automaton commented May 20, 2022

dali-automaton commented May 20, 2022

Add `fn.experimental.audio_resample` GPU #3911

Add `fn.experimental.audio_resample` GPU #3911

mzient May 20, 2022 •

edited

Loading