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

[FEATURE REQUEST] append and vstack #671

Closed
kwagyeman opened this issue Jul 9, 2024 · 10 comments
Closed

[FEATURE REQUEST] append and vstack #671

kwagyeman opened this issue Jul 9, 2024 · 10 comments
Labels
enhancement New feature or request

Comments

@kwagyeman
Copy link

Describe the solution you'd like
I would like if append() and vstack() could be implemented.

Also if this limitation could be removed.

Indices can also be a list of Booleans. By using a Boolean list, we can select those elements of an array that satisfy a specific condition. At the moment, such indexing is defined for row vectors only; when the rank of the tensor is higher than 1, the function raises a NotImplementedError exception, though this will be rectified in a future version of ulab.

Additional context
I was trying to implement non-max-suppression using Ulab and ran into the lack of these features complicating things for me.

Happy to finance this work.

@kwagyeman kwagyeman added the enhancement New feature or request label Jul 9, 2024
@v923z
Copy link
Owner

v923z commented Jul 9, 2024

append should be relatively straightforward, and others might also find it useful, so I would be happy to add it.

Reading the documentation of vstack, it seems to me that it's only a shortcut to what you can already do: concatenate has already been implemented. If that doesn't match your requirements, we could discuss the addition of vstack.

Also if this limitation could be removed.

Do you mean the fact that Boolean indices are supposed to be 1D arrays, or something else?

@v923z v923z changed the title [FEATURE REQUEST] [FEATURE REQUEST] appends and vstack Jul 9, 2024
@v923z v923z changed the title [FEATURE REQUEST] appends and vstack [FEATURE REQUEST] append and vstack Jul 9, 2024
@kwagyeman
Copy link
Author

kwagyeman commented Jul 9, 2024

Reading the documentation of vstack, it seems to me that it's only a shortcut to what you can already do: concatenate has already been implemented. If that doesn't match your requirements, we could discuss the addition of vstack.

It's more about making it easier to use desktop code directly without having to modify it too much. vstack would just be a wrapper around concatenate.

Do you mean the fact that Boolean indices are supposed to be 1D arrays, or something else?

If I try to use a 1D array of booleans to select rows from a 2D array you get a crash right now. Not a Not implemented error.

np_boxes is a 2D array of (x, y, w, h, score, class).

filtered_boxes = np_boxes[np_boxes[:, 4] > 0.0]

The above crashes.

@v923z
Copy link
Owner

v923z commented Jul 9, 2024

If I try to use a 1D array of booleans to select rows from a 2D array you get a crash right now. Not a Not implemented error.

np_boxes is a 2D array of (x, y, w, h, score, class).

filtered_boxes = np_boxes[np_boxes[:, 4] > 0.0]

The above crashes.

OK, this is a different issue then, thanks for bringing it up! I opened a ticket here #672, so that it can be addressed.

@kwagyeman
Copy link
Author

@v923z - Any updates on this?

@v923z
Copy link
Owner

v923z commented Oct 7, 2024

Merging the implementation of take would address the issue raised here, too, right?

@kwagyeman
Copy link
Author

kwagyeman commented Oct 7, 2024

This is more of just a wrapper around concatenate so that append/vstack work. Would be great to have them as normal so desktop code just works.

Yes, otherwise, take addresses the other issue in the thread.

@v923z v923z mentioned this issue Oct 7, 2024
@v923z
Copy link
Owner

v923z commented Oct 7, 2024

Wrappers should/could be added in https://github.com/v923z/micropython-ulab/tree/master/snippets. That has the advantage that that doesn't consume irrecoverable flash space.

@kwagyeman
Copy link
Author

If it's controlled by an ifdef that works for me. It can be off by default and I can enable it for the OpenMV Cams.

@v923z
Copy link
Owner

v923z commented Oct 7, 2024

Everything in https://github.com/v923z/micropython-ulab/tree/master/snippets is in the python layer, so you can simply decide, whether you ship the wrapper with your firmware.

@v923z
Copy link
Owner

v923z commented Oct 9, 2024

Addressed by #688.

@v923z v923z closed this as completed Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants