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

resample fails on axis >1 #73

Closed
Jacob-Stevens-Haas opened this issue Feb 10, 2020 · 5 comments · Fixed by #90
Closed

resample fails on axis >1 #73

Jacob-Stevens-Haas opened this issue Feb 10, 2020 · 5 comments · Fixed by #90
Labels
Milestone

Comments

@Jacob-Stevens-Haas
Copy link

Jacob-Stevens-Haas commented Feb 10, 2020

resampy version: 0.2.1
Python: 3.7.3
numpy version: 1.16.2
Windows 10

resampy.resample returns array of zeros of appropriate size when axis parameter is greater than 1 (tested for axis=2 and axis=3). See minimal working example below:

>>> rand_arr = np.random.rand(3,4,5,100)
>>> resampled_arr = resampy.resample(rand_arr, 100, 24, axis=3)
>>> resampled_t_arr = resampy.resample(np.transpose(rand_arr), 100, 24, axis=0)
>>> print(resampled_arr.sum())
0.0
>>> print(resampled_t_arr.sum())
718.0698928346849

Believe it's due to the lines:

    x_2d = x.swapaxes(0, axis).reshape((x.shape[axis], -1))
    y_2d = y.swapaxes(0, axis).reshape((y.shape[axis], -1))
    resample_f(x_2d, y_2d, sample_ratio, interp_win, interp_delta, precision)

I can look into submitting a PR if it's simple and desirable?

@bmcfee
Copy link
Owner

bmcfee commented Feb 11, 2020

Thanks for noting this. I'll try to make some time to look into it later in the week, but for now I'll confirm that i see the same behavior as your example.

You're almost certainly correct that this is due to the reshaping hackery we do to keep the resampling output arrays memory-contiguous and easy to compute. I don't see an obvious reason why this shouldn't work though.

@bmcfee bmcfee added the bug label Jun 11, 2022
@bmcfee bmcfee added this to the 0.3.0 milestone Jun 11, 2022
@bmcfee
Copy link
Owner

bmcfee commented Jun 17, 2022

Finally circling back on this - it's amazing how many issues / PRs were abandoned in early 2020.

I'm still not sure why this is happening, and suspect it may be a strange interaction with numba. FWIW we use similar shape manipulation logic in librosa (without numba) all the time and have no problems like this.

@bmcfee
Copy link
Owner

bmcfee commented Jun 17, 2022

Ok, I've tracked this one down, but I'm not sure how to resolve it.

Based on the notes in the reshape documentation, it turns out that y_2d is indeed invoking a copy when the input array is of dimension 3 or greater. When this happens, the resample_f function is writing into a copy of the output buffer, rather than a view of the output buffer.

This 2d-reshape business was all to get around limitations of numba array indexing. However, I just tried it out and it seems that this is no longer necessary. I'll have to work out exactly what version of numba made this possible, but I think we should be able to sort this out and simplify things a bit.

@Jacob-Stevens-Haas
Copy link
Author

Thanks! I had ended up forgetting about this too, but it's cool to see things come full circle!

@bmcfee
Copy link
Owner

bmcfee commented Jun 17, 2022

Fix should go out in the 0.3 release, hopefully some time next week.

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

Successfully merging a pull request may close this issue.

2 participants