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 np.tile #5785

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Implement np.tile #5785

wants to merge 8 commits into from

Conversation

AndrewEckart
Copy link
Contributor

@AndrewEckart AndrewEckart commented May 31, 2020

Add basic support for np.tile, limited to when the first input (the tiles) have at most 1 dimension. Includes documentation and tests.

I'd like to support higher-dimensional inputs, but I spent a while trying to figure out how to dynamically allocate a new tuple at runtime for the output shape with little success (the number of dimensions needs to be equal to max(len(reps), a.ndim). It seems like the only option is context.make_tuple using the lower_builtin API, but that didn't seem like a great idea. A possible workaround is using a list/array for the shape, but unfortunately that's not yet supported by np.reshape or array.reshape.

Fixes #2327
Fixes #3456

Add basic support for np.tile, limited to when the first input (the tiles) have at most 1 dimension. Includes documentation and tests. Could easily scale to arbitrary-dimensional inputs if the tuple constructor were supported (so the output shape could be computed at runtime).

Fixes numba#2327
Fixes numba#3456
@stuartarchibald
Copy link
Contributor

Thanks for the patch. There's a few tricks to do tuple generation. Do you want to perhaps get this restricted version in first and then work on expanding it?

@AndrewEckart
Copy link
Contributor Author

@stuartarchibald Sure, if you think that would be best. What are the tricks?

@AndrewEckart AndrewEckart marked this pull request as ready for review June 1, 2020 14:02
@AndrewEckart AndrewEckart changed the title [WIP] Implement np.tile Implement np.tile Jun 12, 2020
docs/source/reference/numpysupported.rst Outdated Show resolved Hide resolved
numba/np/arraymath.py Outdated Show resolved Hide resolved
numba/np/arraymath.py Outdated Show resolved Hide resolved
numba/np/arraymath.py Outdated Show resolved Hide resolved
@mishana
Copy link

mishana commented Jun 12, 2020

Btw, shouldn't this be implemented in numba.np.arrayobj instead? (np.repeat is implemented there)

@AndrewEckart
Copy link
Contributor Author

Btw, shouldn't this be implemented in numba.np.arrayobj instead? (np.repeat is implemented there)

Yeah, probably. I think my rationale was that repeat is available as a method on ndarray and arraymath had this "Building matrices" section with functions that also weren't really computational. But I think tile is more similar to hstack, vstack, etc. as well as repeat so you're right that it probably fits better in arrayobj.

numba/np/arrayobj.py Outdated Show resolved Hide resolved
@stuartarchibald stuartarchibald added this to the PR Backlog milestone Jun 18, 2020
@jdtuck
Copy link

jdtuck commented Oct 26, 2020

what is the status on merging this in, this feature would be great

@ngodber
Copy link

ngodber commented Apr 8, 2021

I'd also like to join the chorus of requestors. Tile is such a useful utility function it can entail quite a bit of refactoring to replace it!

@gmarkall
Copy link
Member

gmarkall commented Apr 8, 2021

After merging master into this branch there is a small conflict which is trivial to resolve, and the added tests of np.tile pass - @AndrewEckart would you like to merge master into your branch and I can add this to the review queue?

@@ -1712,7 +1712,7 @@ def np_triu_indices_from_impl(arr, k=0):
return np_triu_indices_from_impl


def _prepare_array(arr):
def _prepare_array(arr) -> np.ndarray:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how this type annotation got in here - my guess is that it was removed some time in the last several months after I originally branched, and then git put it back when I merged master into the feature branch.

@gmarkall
Copy link
Member

gmarkall commented Apr 8, 2021

Thanks for the update! I've added this to the review queue, and put it in the 0.54 milestone to make sure it doesn't get passed over or omitted from consideration for the next release without proper consideration.

@ngodber
Copy link

ngodber commented Apr 9, 2021

Excited to use this in anger! One less impediment to decorating a function and being off to the races. Thanks a lot guys, very much appreciated!

@raghavduddala
Copy link

It would be great to have this functionality. Is this functionality going to be merged sometime soon? Seems like this PR was added for a review for 0.54 release. Is anyone still working on this or this PR stale?

@AlessandroFazio
Copy link

Hello, it would be useful to have a built-in implementation of np.tile. Hope it will be merge in the future.

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

Successfully merging this pull request may close these issues.

Support numpy.tile Please add np.tile implementation in nopython mode
9 participants