Skip to content

make dimension checker of scatter_add_ consistent with scatter_ #5659

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
soumith merged 2 commits into from
Mar 13, 2018
Merged

make dimension checker of scatter_add_ consistent with scatter_ #5659

soumith merged 2 commits into from
Mar 13, 2018

Conversation

@Officium
Copy link
Contributor

@Officium Officium commented Mar 9, 2018

fix issue #5405

@vadimkantorov
Copy link
Contributor

vadimkantorov commented Mar 9, 2018

this only fixes dimension checking, right? (not broadcasting or support for scalar source)

@Officium
Copy link
Contributor Author

Officium commented Mar 10, 2018

Yes. If we request this feature, I would like to do it. @colesbury

zou3519
zou3519 reviewed Mar 12, 2018
View reviewed changes
Copy link
Contributor

@zou3519 zou3519 left a comment

Choose a reason for hiding this comment

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

@Officium thank you for your PR! It looks good for the most part. I had one minor comment about the code duplication.

aten/src/TH/generic/THTensorMath.c Outdated
// Tests:
// 1. index->size[d] <= src->size[d] for all d
// 2. index->size[d] <= real->size[d] for all d != dim
#define TH_TENSOR_DIM_APPLY3_SIZE_SCATTER(TENSOR1, TENSOR2, TENSOR3, DIMENSION) \

This comment was marked as off-topic.

Sign in to view

@Officium
Copy link
Contributor Author

Officium commented Mar 13, 2018 •
edited
Loading

@zou3519 Thank you for review. I fix it.

@Officium
Copy link
Contributor Author

Officium commented Mar 13, 2018 •
edited
Loading

@pytorchbot test this please

@soumith
Copy link
Contributor

soumith commented Mar 13, 2018

@pytorchbot retest this please

@soumith soumith merged commit f377159 into pytorch:master Mar 13, 2018
@soumith
Copy link
Contributor

soumith commented Mar 13, 2018

thanks @Officium !

@soumith soumith mentioned this pull request Mar 13, 2018
Open
@Officium Officium mentioned this pull request Mar 13, 2018
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zou3519 zou3519 zou3519 left review comments

Assignees

No one assigned

Labels

open source

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Officium @vadimkantorov @soumith @zou3519 @ezyang