Tests/integration tests by bglid · Pull Request #331 · bglid/adaptif

bglid · 2026-10-04T15:29:28Z

Description

Creates integration tests for adaptif. See discussion comment below.

Changes:

Adds CI job for pre-processing and caching MS dataset. Called by other CI jobs
updates .gitignore
Moves python unit tests under python/
Adds tests/setup_integration_data.py for setting up noisy speech dataset for integration tests
Adds python and rust integration tests + utils, that mirror each other, in their respective tests/ directory
Updates Makefile with commands for running different test-suite types

Related Issue

Closes #322

bglid · 2026-10-04T15:29:38Z

Discussion

This isn't finished @elnaske. So there are a few things I want to go through:

Firstly, what do you think about the organization? Does it make sense to keep a utils in each integration dir?
For the Rust integration tests, what do you think about adding a dev-dependency for loading .wav files? I was looking at hound. This is why it's currently some junk (but indicative) signals
It'd be pretty easy to refactor the current 2 integration test functions into one. Thoughts on doing that, or keeping them separated?
Thoughts on the Makefile updates? I can't tell if it's too much? Also, I considered adding in generating the test data, but again didn't know if it was overkill.
I went back and forth about having the args for the metrics in the rust tests be newtypes or not. There isn't a desired signal newtype currently, and making one for this might be a bit much, so I don't know if repurposing the noise reference one for that arg is really correct.
Lastly, thoughts on how to approach integration tests for rust in way that's parameterized like the python ones? I am thinking another declarative macro here.

Also, just any normal PR feedback too. I figure since there's a lot going on here probably best to go through it earlier than later.

bglid · 2026-10-04T15:38:49Z

+    (clean, noisy, noise_ref)
+}
+
+#[allow(clippy::unwrap_used, clippy::indexing_slicing, reason = "Tests")]


Whoops need to remove index slicing here

bglid · 2026-10-04T15:39:02Z

+pub fn mse(desired: &[f64], input: &[f64]) -> f64 {
+    desired
+        .iter()
+        .zip(input)
+        .map(|(d, i)| (d - i).powi(2))
+        .sum::<f64>()
+        / desired.len().to_f64().unwrap()
+}


Need to add comments/docs to these

bglid · 2026-10-04T15:40:09Z

+// NOTE: PLEASE READ / TODO
+// This one function is slop generated and is going to be replaced
+// -> by a real .wav loader. Demo purposes. Will be removed


Disclosing the nonsense here. Chose to do this to avoid bottlenecking this overall PR for something that is going to change. I also didn't want to add hound and build around it without talking about it first.

I'm fine with adding it as a dev dependency

bglid · 2026-10-04T15:40:22Z

+    # NOTE: Microsoft has a bug in their repo. See: https://github.com/microsoft/MS-SNSD/issues/16
+    # The fix is essentially to fix the '*_snrlevels' typecast as int
+    # See: https://github.com/microsoft/MS-SNSD/pull/11


Worth the read lol

tbh if they're not maintaining it anyway we could just fork the repo and apply the fix ourselves

bglid · 2026-10-04T15:40:50Z

 requires-python = ">=3.10"
 dependencies = [
    "numpy>=1.21.0,<2.0.0",
+    "soundfile>=0.14.0",


Same thing with hound kind of goes for this ^

If it's only used for the tests then it should be a dev dependency

elnaske

Firstly, what do you think about the organization? Does it make sense to keep a utils in each integration dir?

I think it's generally fine. My only concern is that we already have src/test_utils.rs, and I want to avoid having two test utils files for Rust. The comparison functions for floats in tests_utils could be replaced with a dev dependency (forgetting the name of the crate rn). We could then move the functions that generate the buffers for comparison into the modules where those buffers are defined (keeping them with #[cfg(test)] though)

It'd be pretty easy to refactor the current 2 integration test functions into one. Thoughts on doing that, or keeping them separated?

I'd merge them, especially if we're going to have tests for all the filter types.

I went back and forth about having the args for the metrics in the rust tests be newtypes or not. There isn't a desired signal newtype currently, and making one for this might be a bit much, so I don't know if repurposing the noise reference one for that arg is really correct.

The desired signal is the cleaned signal right? In that case we do have a type for it, but it's called OutputSignal. And I think using newtypes would be a good idea if only to make sure the tests are actually correct.

Lastly, thoughts on how to approach integration tests for rust in way that's parameterized like the python ones? I am thinking another declarative macro here.

Yeah, macros would be the best solution AFAIK

Think I addressed the other points in the comments

elnaske · 2026-10-04T16:05:08Z

 requires-python = ">=3.10"
 dependencies = [
    "numpy>=1.21.0,<2.0.0",
+    "soundfile>=0.14.0",


If it's only used for the tests then it should be a dev dependency

elnaske · 2026-10-04T16:08:33Z

Since most of this is running terminal commands, do you think you could make it a bash file instead?

Yeah that's a good call I'll change that

elnaske · 2026-10-04T16:10:13Z

+    # NOTE: Microsoft has a bug in their repo. See: https://github.com/microsoft/MS-SNSD/issues/16
+    # The fix is essentially to fix the '*_snrlevels' typecast as int
+    # See: https://github.com/microsoft/MS-SNSD/pull/11


tbh if they're not maintaining it anyway we could just fork the repo and apply the fix ourselves

elnaske · 2026-10-04T16:17:12Z

+// NOTE: PLEASE READ / TODO
+// This one function is slop generated and is going to be replaced
+// -> by a real .wav loader. Demo purposes. Will be removed


I'm fine with adding it as a dev dependency

elnaske · 2026-10-04T16:24:22Z

+#![allow(
+    clippy::tests_outside_test_module,
+    reason = "This crate is exclusively an integration test target"
+)]


You'll probably want to mark this module with #[cfg(test)]. Should get rid of the lint warning and ensures that it's not compiled in release mode.

Actually looked up the docs and they don't have it, so idk

Ik this seems like a weird gap

elnaske · 2026-10-04T16:31:08Z

Do we even need a main.rs? Docs for integration tests don't have it.

Let me try without it. I think I was getting weird errors beforehand because of it being nested in tests/rust as opposed to just tests/

elnaske · 2026-10-04T17:09:38Z

Actually checked out the branch locally and it looks like the data directory is over 5 GB. In that case, wouldn't it make more sense to keep a few wav files as fixtures?

elnaske · 2026-10-04T17:27:42Z

Also the step size, at least for LMS is pretty low. I tried 0.01 like in the tests and you can barely hear a difference. Jacking it up to at least 0.1 works much better

Edit: Checked out the other ones too. Can't get good params for NLMS and Block LMS, so we might have to double check the source code for correctness (or it could be that they just don't work well for this one signal, idk). RLS works fine though.

Edit 2: Since the tests pass anyway, I think it might be a good idea to set minimum thresholds for the SNR. I.e. there needs to be a minimum improvement of x%. Or is that what the MSE tests are for, to check that the output is closer to the clean signal than then the input?

elnaske · 2026-10-04T18:04:49Z

Another thing I found: running uv run pytest directly (not from make) returns some ModuleNotFoundErrors for the integration tests. Also we could set testpaths in pyproject.toml under the pytest options so it only checks the dirs we want

bglid · 2026-10-04T18:09:43Z

The desired signal is the cleaned signal right? In that case we do have a type for it, but it's called OutputSignal. And I think using newtypes would be a good idea if only to make sure the tests are actually correct.

Out atm so only can respond to this one but the desired signal wrt to these tests the desired signal would be the OG clean signal.

elnaske · 2026-10-04T18:35:00Z

Out atm so only can respond to this one but the desired signal wrt to these tests the desired signal would be the OG clean signal.

In that case I'd just use a slice. No use in adding new types if they're only for tests imo

Edit: In the legacy Python code, the docstring for FilterModel.filter() says this:

d (NDArray[np.float64]):
    "Desired Signal", which in the ANC use-case is the noisy input signal.

bglid · 2026-10-05T11:01:50Z

Out atm so only can respond to this one but the desired signal wrt to these tests the desired signal would be the OG clean signal.

In that case I'd just use a slice. No use in adding new types if they're only for tests imo

Edit: In the legacy Python code, the docstring for FilterModel.filter() says this:
d (NDArray[np.float64]):
    "Desired Signal", which in the ANC use-case is the noisy input signal.

In that case I'll change the name in the integration tests to original_wav, essentially that's all it is in these tests. It's just for calculating the MSE and SNR. In adaptive filter lit. they use desired signal as the label for the signal that's attempting to be approximated. It's confusing lol

bglid · 2026-10-05T11:07:24Z

Also the step size, at least for LMS is pretty low. I tried 0.01 like in the tests and you can barely hear a difference. Jacking it up to at least 0.1 works much better

Edit: Checked out the other ones too. Can't get good params for NLMS and Block LMS, so we might have to double check the source code for correctness (or it could be that they just don't work well for this one signal, idk). RLS works fine though.

Edit 2: Since the tests pass anyway, I think it might be a good idea to set minimum thresholds for the SNR. I.e. there needs to be a minimum improvement of x%. Or is that what the MSE tests are for, to check that the output is closer to the clean signal than then the input?

This points to we may want to just have a few pre-set wavs to test on where we know how it'll behave. One of the characteristics of adaptive filters is that they can be finicky, signal/noise dependent too. You'll find that the $\mu can sometimes need to be pretty low. Idk if you recall, in that example I presented with that bela board, the input params for an NLMS filter was:

	// NLMS filter
	gNLMS.setup(32, 0.0001f, 0.000001);    // window_size, mu, eps

Sometimes they can take a second or two to converge. It's easy to accidentally diverge on them.

bglid · 2026-10-05T11:09:26Z

Actually checked out the branch locally and it looks like the data directory is over 5 GB. In that case, wouldn't it make more sense to keep a few wav files as fixtures?

Wrt to other comment, I think this is smart to do. I suppose what's the best way to do this? I don't want git to track it, so what about forking the MS repo, generating a handful of .wav files, then removing most of the unneeded slop from the repo, then having the .sh setup file pull those files into the /data directory upon command?

elnaske · 2026-10-05T15:55:18Z

Wrt to other comment, I think this is smart to do. I suppose what's the best way to do this? I don't want git to track it, so what about forking the MS repo, generating a handful of .wav files, then removing most of the unneeded slop from the repo, then having the .sh setup file pull those files into the /data directory upon command?

Actually I was just thinking keeping them as fixtures in the repo. We only need a few short wav files, so it shouldn't take up to much space.

elnaske · 2026-10-05T16:11:48Z

This points to we may want to just have a few pre-set wavs to test on where we know how it'll behave. One of the characteristics of adaptive filters is that they can be finicky, signal/noise dependent too. You'll find that the $\mu can sometimes need to be pretty low. Idk if you recall, in that example I presented with that bela board, the input params for an NLMS filter was:
	// NLMS filter
	gNLMS.setup(32, 0.0001f, 0.000001);    // window_size, mu, eps
Sometimes they can take a second or two to converge. It's easy to accidentally diverge on them.

In that case we'll want to manually test parameters to find ones that work well with the input. I see that the Python CI is currently failing because the SNR after block LMS is slightly worse, so we'll want good parameters to avoid this. I want the results of the integration to tests to be really clear cut so that we don't run the risk of them failing due to chance.

bglid added 18 commits September 30, 2026 07:56

integration init with gitignore updates.

a46dc45

Moving tests and setting up Make for integration vs unit tests

0de305c

updating the gitignore

6ae358f

updating the gitignore

198b27e

fixing MS bug

e0ca2f4

cleanup

826d7be

fixing tests gitignore and adding soundfile for data load

ff93933

first working integration tests python

af35efa

Adding data preparation and integration job that's called

03ac398

fixing ubuntu run dep

0cf579f

Adjusting CIs smh

6c69b34

Pinning rust version and handling mr clippy

6ae3c89

Working python int tests, need to investigate more

29a29a6

Removing block lms for now

51f6cf1

Merge branch 'main' into tests/integration-tests

4c5c563

reverting to explicit commands for now in CIs

12b8d21

working test rust integration tests

343861e

remove that lint now

708da78

bglid commented Oct 4, 2026

View reviewed changes

elnaske reviewed Oct 4, 2026

View reviewed changes

bglid added 2 commits October 5, 2026 07:36

switching rust pinning to rust-toolchain.toml

7b0a784

cleaning makefile

86124d6

Conversation

bglid commented Oct 4, 2026

Description

Changes:

Related Issue

Uh oh!

bglid commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Discussion

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bglid Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elnaske left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elnaske commented Oct 4, 2026

Uh oh!

elnaske commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elnaske commented Oct 4, 2026

Uh oh!

bglid commented Oct 4, 2026

Uh oh!

elnaske commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bglid commented Oct 5, 2026

Uh oh!

bglid commented Oct 5, 2026

Uh oh!

bglid commented Oct 5, 2026

Uh oh!

elnaske commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elnaske commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

bglid commented Oct 4, 2026 •

edited

Loading

bglid Oct 4, 2026 •

edited

Loading

elnaske left a comment •

edited

Loading

elnaske commented Oct 4, 2026 •

edited

Loading

elnaske commented Oct 4, 2026 •

edited

Loading

elnaske commented Oct 5, 2026 •

edited

Loading

elnaske commented Oct 5, 2026 •

edited

Loading