Skip to content

Implement broadcast routines #759

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.

Sign up for GitHub

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

Jump to bottom
Merged
bryevdv merged 39 commits into from
Jun 23, 2023
Merged

Implement broadcast routines #759

bryevdv merged 39 commits into from
Jun 23, 2023

Conversation

@bryevdv
Copy link
Contributor

@bryevdv bryevdv commented Jan 20, 2023 •
edited
Loading

Supersedes #458

I was not able to rebase quite a delicately as I'd hoped, but simply squashed the branch and reset the commit author for work to date.

@bryevdv bryevdv requested a review from magnatelee January 20, 2023 19:19
@bryevdv bryevdv added the category:new-feature PR introduces a new feature and will be classified as such in release notes label Jan 20, 2023
@bryevdv bryevdv mentioned this pull request Jan 20, 2023
Closed
@marcinz marcinz changed the base branch from branch-22.12 to branch-23.03 January 26, 2023 00:55
@manopapad manopapad requested review from manopapad and removed request for magnatelee February 6, 2023 19:26
@ipdemes
Copy link
Contributor

ipdemes commented Feb 6, 2023

Is there an ETA for merging this PR? I would like to use broadcasting routines from here in my "vectorize" implementation

@bryevdv
Copy link
Contributor Author

bryevdv commented Feb 6, 2023 •
edited
Loading

I've been directed to focus on other things, so probably not this release cycle unless someone else picks it up (it is cleaned up some and up to date with latest now, at least)

@ipdemes
Copy link
Contributor

ipdemes commented Feb 8, 2023

OK, I will work around this in my branch then

@marcinz marcinz changed the base branch from branch-23.03 to branch-23.05 March 6, 2023 20:49
@bryevdv
Copy link
Contributor Author

bryevdv commented Mar 14, 2023 •
edited
Loading

@manopapad @magnatelee FYI I have updated and rebased this on branch-23.05 and all tests are passing locally. Let me know any thoughts about the current state of things.

Edit: keeping rebased to current periodically

@bryevdv
Copy link
Contributor Author

bryevdv commented May 11, 2023

@manopapad @magnatelee any thoughts on this one? I am happy to try to drive it to finish with some review/guidance.

manopapad
manopapad requested changes May 16, 2023
View reviewed changes
Copy link
Contributor

@manopapad manopapad left a comment

Choose a reason for hiding this comment

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

I still need to go over the tests, but this covers my observations on the code itself.

@marcinz marcinz changed the base branch from branch-23.05 to branch-23.07 May 18, 2023 20:27
manopapad
manopapad requested changes Jun 13, 2023
View reviewed changes
manopapad
manopapad approved these changes Jun 15, 2023
View reviewed changes
Copy link
Contributor

@manopapad manopapad left a comment

Choose a reason for hiding this comment

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

Everything looks good to me. Please review/edit the tests I added, and merge when everything looks good.

bryevdv
bryevdv commented Jun 20, 2023
View reviewed changes
@manopapad
Copy link
Contributor

manopapad commented Jun 21, 2023 •
edited
Loading

Noting that the latest CI failure with AttributeError: 'numpy._ArrayFunctionDispatcher' object has no attribute '_cunumeric' is happening on other PRs as well, and doesn't seem to reproduce on my local machine, so it's likely not tied to the changes in this PR, but more likely it's due to some change in the CI setup.

@bryevdv
Copy link
Contributor Author

bryevdv commented Jun 21, 2023

@manopapad Numpy 1.25 was released 4 days ago I am guessing they added something that causes this issue. Looking into it

@bryevdv
Copy link
Contributor Author

bryevdv commented Jun 23, 2023

Docs issue is unrelated, will fix in a separate PR

@bryevdv bryevdv merged commit 07fb5f2 into nv-legate:branch-23.07 Jun 23, 2023
@bryevdv bryevdv deleted the bv/broadcast branch June 23, 2023 23:18
@bryevdv bryevdv mentioned this pull request Jan 25, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@manopapad manopapad manopapad approved these changes

Assignees

No one assigned

Labels

category:new-feature PR introduces a new feature and will be classified as such in release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@bryevdv @ipdemes @manopapad @sbak5