Add Ray tracing method for RIR (#2850) #3234

nateanl · 2023-04-03T19:29:31Z

Summary:
This PR adds the ray_tracing() helper to compute a RIR (part of #2624). The implementation is heavily based on pyroomacoustics.

Pull Request resolved: #2850

Differential Revision: D41764237

Pulled By: nateanl

facebook-github-bot · 2023-04-03T19:30:07Z

This pull request was exported from Phabricator. Differential Revision: D41764237

nateanl · 2023-04-05T22:14:50Z

cc @fakufaku @JupiterEthan ray_tracing function should be ready, would like to hear thoughts from you. Thanks!

mthrok · 2023-04-12T01:34:21Z

torchaudio/csrc/ray_tracing.cpp

+      const torch::Tensor& _absorption,
+      const torch::Tensor& _scattering,
+      const torch::Tensor& _normal,
+      const torch::Tensor& _origin)
+      : absorption(std::move(_absorption)),
+        reflection((scalar_t)1. - _absorption),
+        scattering(std::move(_scattering)),
+        normal(std::move(_normal)),
+        origin(std::move(_origin)) {}


Applying std::move to const reference is strange.
You can probably remove std::move and copy construct them.

mthrok · 2023-04-12T01:34:35Z

torchaudio/csrc/ray_tracing.cpp

+  torch::Tensor get_absorption() {
+    return absorption;
+  }


seems not used.

mthrok · 2023-04-12T01:35:11Z

torchaudio/csrc/ray_tracing.cpp

+  torch::Tensor get_reflection() {
+    return reflection;
+  }
+  torch::Tensor get_scattering() {
+    return scattering;
+  }


Probably, making it a attribute and accessing directly would do.

mthrok · 2023-04-12T01:38:34Z

torchaudio/csrc/ray_tracing.cpp

+    false; // TODO: remove this once hybrid method is supported
+const int ISM_ORDER = 10; // TODO: remove this once ISM method is supported
+#define EPS ((scalar_t)(1e-5))
+#define VAL(t) (*((t).template data_ptr<scalar_t>()))


I don't think this macro is necessary. Simply calling item method should do.

mthrok · 2023-04-12T01:45:40Z

torchaudio/csrc/ray_tracing.cpp

+  auto num_mics = mic_array.size(0);
+  auto num_bands = absorption.size(0);


Cannot call size(0), without first checking the ndim >= 1

mthrok · 2023-04-12T01:47:01Z

torchaudio/csrc/ray_tracing.cpp

+      scalar_t _energy_thres,
+      scalar_t _time_thres,
+      scalar_t _hist_bin_size)
+      : room(_room),


the dimension of room is not checked.

mthrok · 2023-04-12T01:47:20Z

torchaudio/csrc/ray_tracing.cpp

+        time_thres(_time_thres),
+        hist_bin_size(_hist_bin_size),
+        max_dist(VAL(room.norm()) + (scalar_t)1.),
+        D(room.size(0)),


D is unbounded. Need validation

Rather, I'd make D a template parameter.

mthrok · 2023-04-12T01:48:39Z

test/torchaudio_unittest/prototype/functional/functional_test_impl.py

+
+        assert hist.shape == expected_shape
+
+    def test_ray_tracing_input_errors(self):


need more of these. currently the implementation does not reject the invalid shapes like empty tensors.

mthrok · 2023-04-12T01:49:07Z

test/torchaudio_unittest/prototype/functional/functional_test_impl.py

+            scattering=scattering,
+        )
+
+        assert hist.shape == expected_shape


Please use self.assertEqual. (Buck does not report numbers with assert)

mthrok · 2023-04-12T01:50:07Z

torchaudio/csrc/ray_tracing.cpp

+  /**
+   * The main (and only) public entry point of this class. The histograms Tensor
+   * reference is passed along and modified in the subsequent private method
+   * calls. This method spawns num_rays rays in all directions from the source
+   * and calls simul_ray() on each of them.
+   */


Please use // for comment so that later it's easy to block the whole thing

mthrok · 2023-04-12T01:53:24Z

torchaudio/csrc/ray_tracing.cpp

+  histograms = histograms.transpose(
+      1, 2); // back into shape (num_mics, num_bands, num_bins)


Can you move the comment to separate line? It's easier to read that way.

mthrok · 2023-04-12T01:53:57Z

torchaudio/csrc/ray_tracing.cpp

+      travel_dist += hit_distance;
+      transmitted *= wall.get_reflection();
+
+      //   Let's shoot the scattered ray induced by the rebound on the wall


Suggested change

// Let's shoot the scattered ray induced by the rebound on the wall

// Let's shoot the scattered ray induced by the rebound on the wall

mthrok · 2023-04-12T01:55:37Z

torchaudio/csrc/ray_tracing.cpp

+const bool IS_HYBRID_SIM =
+    false; // TODO: remove this once hybrid method is supported


please move the comment to separate line

pytorch-bot · 2023-09-08T19:38:28Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/audio/3234

📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❌ 7 New Failures

As of commit ca69eaa with merge base 5e893d6 ():

NEW FAILURES - The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Summary: This PR adds the `ray_tracing()` helper to compute a RIR (part of pytorch#2624). The implementation is heavily based on `pyroomacoustics`. Differential Revision: D41764237 Pulled By: nateanl

facebook-github-bot · 2023-09-08T19:38:36Z

This pull request was exported from Phabricator. Differential Revision: D41764237

mthrok · 2023-09-11T19:52:30Z

torchaudio/csrc/ray_tracing.cpp

+      }
+
+      if (intersection_found) {
+        next_wall_index = 2 * d[0] + ind_inc;


@nateanl I feel like this should be d[0] + 2 * ind_inc. Do we have test to verify this is correct?

@nateanl Actually, this is correct for 3D but not for 2D. Because for 2D the order of wall is SENW, while for 3D it's WESNCF.

Summary: This PR adds the `ray_tracing()` helper to compute a RIR (part of pytorch#2624). The implementation is heavily based on `pyroomacoustics`. Differential Revision: D41764237 Pulled By: nateanl

facebook-github-bot · 2023-09-11T22:24:17Z

This pull request was exported from Phabricator. Differential Revision: D41764237

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174

Taken from pytorch#3234

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

Taken from #3234

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

Summary: Revamped version of #3234 (which was also revamp of #2850)

facebook-github-bot added CLA Signed fb-exported labels Apr 3, 2023

mthrok reviewed Apr 12, 2023

View reviewed changes

nateanl force-pushed the export-D41764237 branch from 0be21fb to 57ddebf Compare September 8, 2023 19:38

nateanl requested a review from a team as a code owner September 8, 2023 19:38

mthrok reviewed Sep 11, 2023

View reviewed changes

Add Ray tracing method for RIR (pytorch#3234)

ca69eaa

Summary: This PR adds the `ray_tracing()` helper to compute a RIR (part of pytorch#2624). The implementation is heavily based on `pyroomacoustics`. Differential Revision: D41764237 Pulled By: nateanl

nateanl force-pushed the export-D41764237 branch from 57ddebf to ca69eaa Compare September 11, 2023 22:24

mthrok mentioned this pull request Sep 12, 2023

WIP: Add Ray Tracing #3604

Closed

mthrok added a commit to mthrok/audio that referenced this pull request Sep 12, 2023

WIP: Add Ray Tracing

d324984

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174

mthrok added a commit to mthrok/audio that referenced this pull request Sep 13, 2023

WIP: Add Ray Tracing (pytorch#3604)

5c3872b

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174

mthrok added a commit to mthrok/audio that referenced this pull request Sep 13, 2023

WIP: Add Ray Tracing (pytorch#3604)

1f77a54

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174

mthrok added a commit to mthrok/audio that referenced this pull request Oct 2, 2023

Add the rest of C++ ray tracing implementation

42c8292

Taken from pytorch#3234

mthrok mentioned this pull request Oct 2, 2023

Add the rest of C++ ray tracing implementation #3630

Merged

mthrok added a commit to mthrok/audio that referenced this pull request Oct 2, 2023

Add the rest of C++ ray tracing implementation

22281d3

Taken from pytorch#3234

mthrok added a commit to mthrok/audio that referenced this pull request Oct 2, 2023

WIP: Add Ray Tracing (pytorch#3604)

7d124e3

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 2, 2023

WIP: Add Ray Tracing (pytorch#3604)

59bc3e6

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 2, 2023

WIP: Add Ray Tracing (pytorch#3604)

8bde7a8

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit that referenced this pull request Oct 2, 2023

Add the rest of C++ ray tracing implementation (#3630)

ff266b1

Taken from #3234

mthrok added a commit to mthrok/audio that referenced this pull request Oct 2, 2023

WIP: Add Ray Tracing (pytorch#3604)

0bd9738

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 2, 2023

WIP: Add Ray Tracing (pytorch#3604)

d6f8171

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 3, 2023

WIP: Add Ray Tracing (pytorch#3604)

98838b8

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 3, 2023

WIP: Add Ray Tracing (pytorch#3604)

db3e607

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 3, 2023

WIP: Add Ray Tracing (pytorch#3604)

804231c

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 3, 2023

WIP: Add Ray Tracing (pytorch#3604)

cd8c482

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 3, 2023

WIP: Add Ray Tracing (pytorch#3604)

4117ac6

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 3, 2023

WIP: Add Ray Tracing (pytorch#3604)

bc50737

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 4, 2023

WIP: Add Ray Tracing (pytorch#3604)

49be591

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 4, 2023

WIP: Add Ray Tracing (pytorch#3604)

e15170d

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 4, 2023

WIP: Add Ray Tracing (pytorch#3604)

69c2ef4

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 4, 2023

WIP: Add Ray Tracing (pytorch#3604)

99f16c6

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 5, 2023

WIP: Add Ray Tracing (pytorch#3604)

87dbc6c

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 5, 2023

WIP: Add Ray Tracing (pytorch#3604)

574ae45

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 5, 2023

WIP: Add Ray Tracing (pytorch#3604)

270e2ce

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 5, 2023

WIP: Add Ray Tracing (pytorch#3604)

ec69786

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 5, 2023

WIP: Add Ray Tracing (pytorch#3604)

f0b2200

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 9, 2023

WIP: Add Ray Tracing (pytorch#3604)

20b92cd

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 9, 2023

WIP: Add Ray Tracing (pytorch#3604)

a39f133

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 9, 2023

Add Ray Tracing (pytorch#3604)

5a49d9c

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok added a commit to mthrok/audio that referenced this pull request Oct 13, 2023

Add Ray Tracing (pytorch#3604)

7cb59c3

Summary: Revamped version of pytorch#3234 (which was also revamp of pytorch#2850) Differential Revision: D49197174 Pulled By: mthrok

mthrok mentioned this pull request Oct 13, 2023

Add Ray Tracing (#3604) #3655

Merged

mthrok added a commit that referenced this pull request Oct 13, 2023

Add Ray Tracing (#3604) (#2850) (#3655)

fa78fb6

Summary: Revamped version of #3234 (which was also revamp of #2850)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Ray tracing method for RIR (#2850) #3234

Add Ray tracing method for RIR (#2850) #3234

nateanl commented Apr 3, 2023

facebook-github-bot commented Apr 3, 2023

nateanl commented Apr 5, 2023

mthrok Apr 12, 2023

mthrok Apr 12, 2023

mthrok Apr 12, 2023

mthrok Apr 12, 2023

mthrok Apr 12, 2023

mthrok Apr 12, 2023

mthrok Apr 12, 2023

mthrok Apr 12, 2023

mthrok Apr 12, 2023

mthrok Apr 12, 2023

mthrok Apr 12, 2023

mthrok Apr 12, 2023

mthrok Apr 12, 2023

mthrok Apr 12, 2023

pytorch-bot bot commented Sep 8, 2023 •

edited

facebook-github-bot commented Sep 8, 2023

mthrok Sep 11, 2023

mthrok Sep 11, 2023

facebook-github-bot commented Sep 11, 2023

		auto num_mics = mic_array.size(0);
		auto num_bands = absorption.size(0);


		assert hist.shape == expected_shape

		def test_ray_tracing_input_errors(self):

		histograms = histograms.transpose(
		1, 2); // back into shape (num_mics, num_bands, num_bins)

	// Let's shoot the scattered ray induced by the rebound on the wall
	// Let's shoot the scattered ray induced by the rebound on the wall

		const bool IS_HYBRID_SIM =
		false; // TODO: remove this once hybrid method is supported

Add Ray tracing method for RIR (#2850) #3234

Are you sure you want to change the base?

Add Ray tracing method for RIR (#2850) #3234

Conversation

nateanl commented Apr 3, 2023

facebook-github-bot commented Apr 3, 2023

nateanl commented Apr 5, 2023

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pytorch-bot bot commented Sep 8, 2023 • edited

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/audio/3234

❌ 7 New Failures

facebook-github-bot commented Sep 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Sep 11, 2023

pytorch-bot bot commented Sep 8, 2023 •

edited