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.

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

Allow silent targets and estimates #95

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

DavidDiazGuerra
Copy link
Contributor

Hello,

I'm proposing this PR following the discussions of #94. With this implementation, mus-eval raises a warning instead of an error if one of the sources or estimates is completely silent and compute_permutation is False (if it is True it raises the same error it used to).

Some important notes:

  • It handles silent sources or estimates in a way equivalent to how the library already handled silent frames: the metric is set to NaN or -Inf in that song and then the song is excluded when computing the average metric of the dataset.
  • This does not modify any of the results that could be computed before, it just allows getting results for inputs that raised errors.

I think this is important for dealing with datasets more complex than MusDB18, where a wider variety of instruments could be available in the whole dataset but not all the instruments might be present in every song (this situation could be quite common, for example, in classical music).

Best,
David

DavidDiazGuerra and others added 2 commits November 9, 2023 11:17
Make sure the output of _safe_db is always a np.float64 even when Inf or NaN so it has the attribute shape
@DavidDiazGuerra
Copy link
Contributor Author

Sorry for the bug in the previous version. Now the only reason the tests are not passed is that test_silent_input is obviously not raising a ValueError. I'm not sure if you would prefer to modify or just remove that test.

@faroit
Copy link
Member

faroit commented Jan 16, 2024

@DavidDiazGuerra maybe modify the tests

@DavidDiazGuerra
Copy link
Contributor Author

Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants