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

[BUG] array must be contiguous #312

Open
zxweed opened this issue Jul 21, 2024 · 3 comments
Open

[BUG] array must be contiguous #312

zxweed opened this issue Jul 21, 2024 · 3 comments
Labels
bug Something isn't working dependencies Issues related to dependencies of plotly-resampler enhancement New feature or request Optimization Aims to make the toolkit more memory / computationally efficient

Comments

@zxweed
Copy link

zxweed commented Jul 21, 2024

The error occurs on FORTRAN-style or structured arrays (where data belonging to one series do not reside in memory continuously)

Reproducing the bug 🔍

import numpy as np
from plotly_resampler import FigureResampler

x = np.arange(1_000_000)
y = np.zeros(shape=(2, k), order='F')
y[0] = (3 + np.sin(x / 200) + np.random.randn(len(x)) / 10) * x / 1_000

fig = FigureResampler(go.Figure())
fig.add_trace(go.Scattergl(name='noisy sine', showlegend=True), hf_x=x, hf_y=y[0])

Error message:

File ~/micromamba/lib/python3.11/site-packages/tsdownsample/downsampling_interface.py:376, in AbstractRustDownsampler.downsample(self, n_out, parallel, *args, **kwargs)
    368 def downsample(self, *args, n_out: int, parallel: bool = False, **kwargs):
    369     """Downsample the data in x and y.
    370 
    371     The x and y arguments are positional-only arguments. If only one argument is
   (...)
    374     considered to be the y-data.
    375     """
--> 376     return super().downsample(*args, n_out=n_out, parallel=parallel, **kwargs)

File ~/micromamba/lib/python3.11/site-packages/tsdownsample/downsampling_interface.py:131, in AbstractDownsampler.downsample(self, n_out, *args, **kwargs)
    129 x, y = self._check_valid_downsample_args(*args)
    130 self._supports_dtype(y, y=True)
--> 131 self._check_contiguous(y, y=True)
    132 if x is not None:
    133     self._supports_dtype(x, y=False)

File ~/micromamba/lib/python3.11/site-packages/tsdownsample/downsampling_interface.py:38, in AbstractDownsampler._check_contiguous(self, arr, y)
     35 if arr.flags["C_CONTIGUOUS"]:
     36     return
---> 38 raise ValueError(f"{'y' if y else 'x'} array must be contiguous.")

ValueError: y array must be contiguous.

Environment information:

  • OS: linux
  • Python environment:
    • Python 3.8+:
    • plotly-resampler environment: Jupyter(lab)
  • plotly-resampler version: 0.10.0

Additional context
It looks like the downsampler can only process strictly contiguous data. It would be good to either make it work with strides or call np.ascontiguousarray after check the C_CONTIGUOUS flag.

@zxweed zxweed added the bug Something isn't working label Jul 21, 2024
@DHRUVCHARNE
Copy link

To fix this issue, you can convert the array to a contiguous array using np.ascontiguousarray function, like this:

y_contiguous = np.ascontiguousarray(y[0])
fig.add_trace(go.Scattergl(name='noisy sine', showlegend=True), hf_x=x, hf_y=y_contiguous)
Alternatively, you can create the original array with a contiguous memory layout by removing the order='F' parameter:
y = np.zeros(shape=(2, k))

@zxweed
Copy link
Author

zxweed commented Aug 28, 2024

of course I can (and I do), but it would be better if it was done automatically inside plotly_resampler (or better, process by strides - conversion will require double memory consumption)

@jonasvdd
Copy link
Member

jonasvdd commented Sep 5, 2024

@jvdd, I think this issue is more related to tsdownsample. Given that you are the lead developer of that package, what are your thoughts on (the feasibility of) using "stride" information to downsample data (instead of the contiguous assumption)?

@jonasvdd jonasvdd added dependencies Issues related to dependencies of plotly-resampler enhancement New feature or request Optimization Aims to make the toolkit more memory / computationally efficient labels Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Issues related to dependencies of plotly-resampler enhancement New feature or request Optimization Aims to make the toolkit more memory / computationally efficient
Projects
None yet
Development

No branches or pull requests

3 participants