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

Fix tensor shapes for elementwise binary operations with broadcasting #738

Closed
wants to merge 12 commits into from

Conversation

soumyac1999
Copy link
Collaborator

@soumyac1999 soumyac1999 commented May 26, 2023

Description of changes:

Example: [16, 1, 2] + [16, 1552, 1] has output shape of [16, 1552, 2] but the existing code was setting the size to [16, 1, 2].

Related Issues:

Linked Issues:

  • Issue #

Issues closed by this PR:

Before merging:

  • Did you update the flexflow-third-party repo, if modifying any of the Cmake files, the build configs, or the submodules?

This change is Reviewable

@soumyac1999 soumyac1999 requested a review from jiazhihao May 26, 2023 07:30
@jiazhihao
Copy link
Collaborator

The PR lgtm. However, it seems the changes do not pass the following test

$EXE "$FF_HOME"/examples/python/keras/elementwise_mul_broadcast.py -ll:py 1 -ll:gpu "$GPUS" -ll:fsize "$FSIZE" -ll:zsize "$ZSIZE" -b ${BATCHSIZE} --only-data-parallel

@soumyac1999 soumyac1999 force-pushed the broadcast_shapes branch 2 times, most recently from 3691d6f to 2dbc18c Compare October 13, 2023 07:46
@soumyac1999 soumyac1999 reopened this Oct 13, 2023
@soumyac1999 soumyac1999 changed the base branch from master to inference October 13, 2023 07:49
@soumyac1999 soumyac1999 requested a review from jiazhihao October 13, 2023 07:52
@soumyac1999
Copy link
Collaborator Author

The PR lgtm. However, it seems the changes do not pass the following test

$EXE "$FF_HOME"/examples/python/keras/elementwise_mul_broadcast.py -ll:py 1 -ll:gpu "$GPUS" -ll:fsize "$FSIZE" -ll:zsize "$ZSIZE" -b ${BATCHSIZE} --only-data-parallel

@jiazhihao, similar fixes were needed in Keras to correctly compute the output shapes. This test now passes (alteast locally, waiting for the CI checks).

@soumyac1999 soumyac1999 requested a review from lockshaw October 13, 2023 08:04
@soumyac1999
Copy link
Collaborator Author

Moved to #1234

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

Successfully merging this pull request may close these issues.

Investigate possible issues in output shape computation in keras/layers/merge.py
2 participants