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.

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

Add Split and BatchNorm to get_op_parameters #360

Merged
merged 5 commits into from
Oct 18, 2022
Merged

Add Split and BatchNorm to get_op_parameters #360

merged 5 commits into from
Oct 18, 2022

Conversation

goliaro
Copy link
Collaborator

@goliaro goliaro commented Oct 18, 2022

Closes #359

lockshaw
lockshaw requested changes Oct 18, 2022
View reviewed changes
Copy link
Collaborator

@lockshaw lockshaw left a comment

Choose a reason for hiding this comment

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

I'm a little confused by what this PR is supposed to do. I fully supporting adding cases for operators that support get_params such as OP_BATCH_NORM, OP_SPLIT, etc. but many of the operators here don't support this method (some don't even appear to have implementations in FF)

@goliaro
Copy link
Collaborator Author

goliaro commented Oct 18, 2022

I'm a little confused by what this PR is supposed to do. I fully supporting adding cases for operators that support get_params such as OP_BATCH_NORM, OP_SPLIT, etc. but many of the operators here don't support this method (some don't even appear to have implementations in FF)

Admittedly, I wasn't entirely sure which operator codes from ffconst.h needed to be added in operator_params.cc. I just removed the ones without implementations, so now the ones left should all support get_params . Let me know if the new code looks good!

@lockshaw lockshaw self-requested a review October 18, 2022 19:12
@lockshaw lockshaw changed the title [Ops] - Added missing ops Add Split and BatchNorm to get_op_parameters Oct 18, 2022
@lockshaw lockshaw merged commit e0b3cf5 into flexflow:master Oct 18, 2022
@goliaro goliaro deleted the fix_no_params_hash_overload branch October 18, 2022 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lockshaw lockshaw lockshaw approved these changes

@reyna-abhyankar reyna-abhyankar Awaiting requested review from reyna-abhyankar

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

No overload of get_params_hash for some operators
2 participants
@goliaro @lockshaw