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

Implement element preserving resize in 1d #2745

Open
tdegeus opened this issue Nov 11, 2023 · 9 comments
Open

Implement element preserving resize in 1d #2745

tdegeus opened this issue Nov 11, 2023 · 9 comments
Labels
Enhancement Help wanted A pull-request is welcomed!

Comments

@tdegeus
Copy link
Member

tdegeus commented Nov 11, 2023

In general shrinking with resize of xt::xtensor<double, 1> does preserve elements. It would be nice to have the option to do so (if the underlying container allows it. E.g.

xt::xtensor<double, 1> a = xt::empty<double>({1000});
...
a.resize({150}, /*preserve*/ true);
@tdegeus tdegeus added Enhancement Help wanted A pull-request is welcomed! labels Nov 11, 2023
@spectre-ns
Copy link
Contributor

spectre-ns commented Nov 11, 2023

@tdegeus Yes I have a couple guys on our staff thinking it worked like std::vector is there a limitation to doing this on the underlying storage?

@spectre-ns
Copy link
Contributor

@tdegeus The bottom level storage is just linear memory. Could it be implemented with a std::vector to propagate STL behaviour to some of the interface methods?

@tdegeus
Copy link
Member Author

tdegeus commented Nov 12, 2023

I think even that mostly the underlying IS std::vector so there are surely many cases where we could fix resize to propagate downstream properties. In addition, it would be nice to add push_back to xt::xtensor<T, 1>. Beyond that, were you thinking about more features?

@spectre-ns
Copy link
Contributor

I think in ND we could implement a greedy concatenate using std::vector::insert to avoid the current performance issues.

@tdegeus
Copy link
Member Author

tdegeus commented Nov 13, 2023

From the implementation I think that the underlying resize is used:

c.resize(size);

So I think that for xtensor and xarray resize is item preserving. Could you comment @JohanMabille ?

Edit I tried, indeed elements are not conserved. Yet, as indicated, at first glance I don't see where elements are modified.

@spectre-ns
Copy link
Contributor

@tdegeus I just read the gitter channel and it sounds like it is deferred to the container implementation. How do you propose ensuring that the implementation be element preserving?

@tdegeus
Copy link
Member Author

tdegeus commented Nov 17, 2023

I have no idea yet ;)

@JohanMabille
Copy link
Member

Actually the default container for xtensor and xarray is not std::vector, but uvector, which does not preserve its elements when resized.

Anyway, the resize with preserving should be independent from the dimension of the tensor, and should not make any assumption regarding the underlying container. One way to implement it would be to compute the "intersection" shape, and then recursively loop over to copy from the first vector to the other one, and eventually swap the vectors.

(I'll try to post some code later)

@spectre-ns
Copy link
Contributor

spectre-ns commented Nov 18, 2023

@JohanMabille why is std::vector<T> not the underlying data structure? I've had a couple colleagues encounter issues with resizing containers expecting the same behaviour as std::vector<T>.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Help wanted A pull-request is welcomed!
Projects
None yet
Development

No branches or pull requests

3 participants