Skip to content

Misc. fixes for new distributed partitioning #397

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
inducer merged 9 commits into from
Nov 28, 2022
Merged

Misc. fixes for new distributed partitioning #397

inducer merged 9 commits into from
Nov 28, 2022

Conversation

@majosm
Copy link
Collaborator

@majosm majosm commented Nov 28, 2022 •
edited by inducer
Loading

Targets #393.

inducer
inducer reviewed Nov 28, 2022
View reviewed changes
@majosm
Copy link
Collaborator Author

majosm commented Nov 28, 2022

(@inducer Didn't you add multiple comments during the meeting? I only see one of them here.)

@majosm
fix conditional
ea63005 
conditional needs to be here because zip won't return the right number of
variables if there aren't any receives; checking if #parts == 1 is not
sufficient though, need to actually check for recvs
@majosm
attempt to fix materialized_ary_to_part_id computation
ee18e54
@majosm
attempt to fix materialized_ary_to_part_id computation (again)
b80875f
@majosm majosm force-pushed the dist-mem-part-2000 branch from 17df3d2 to b80875f Compare November 28, 2022 22:15
@inducer
Copy link
Owner

inducer commented Nov 28, 2022

(@inducer Didn't you add multiple comments during the meeting? I only see one of them here.)

I did, but I deleted them when they became moot when we discovered that things were even wrong-er.

@inducer inducer merged commit fda3964 into inducer:dist-mem-part-2000 Nov 28, 2022
@inducer
Copy link
Owner

inducer commented Nov 28, 2022

Thanks! I've read through these changes, and they make sense to me. Would you be opposed to me squashing those into the existing commits in #393?

@majosm
Copy link
Collaborator Author

majosm commented Nov 29, 2022

Thanks! I've read through these changes, and they make sense to me. Would you be opposed to me squashing those into the existing commits in #393?

Sure, no objections.

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

Reviewers

@inducer inducer inducer left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@majosm @inducer