[Graph C API] Remove GrapplerItemMap #48385
Conversation
|
Hi @penpornk @ezhulenev, please help to have a review. Thanks! |
penpornk
left a comment
There was a problem hiding this comment.
Thank you for the PR! Could you please split them into two separate PRs? That way if one thing failed a test, the other wouldn't get reverted too. It would make it easier to debug as well.
The warning message PR got reverted because it broke an internal test. So if we simply bring it back it will get reverted again. I'll have to find out why first.
|
OK. Now this PR is only targeting removing global GrapplerItemMap. |
penpornk
left a comment
There was a problem hiding this comment.
Thank you again for the PR! :)
|
@kulinseth FYI. We removed GrapplerItem map and just passed the GrapplerItem directly to the optimize function. |
Thanks! for letting us know @penpornk |
|
Hi @ShengYang1 , thanks for the PR. Looks like this wasn't part of the v2.5 release. Kulin |
|
That was my fault. I got feedback to remove the map about a week before the branch cut, but I was worried it could complicate our already tight PluggableDevice-related PR merge/test schedule. So I waited until after 2.5 branch cut. And since cherrypick was technically for bug fixes, I didn't cherrypick it in. Sorry for making diverge code paths! :'( |
No issues, thanks for the clarification @penpornk . The only reason I brought up was that since there is an API change, it won't be a breaking change for v2.5 but for TF master we would at somepoint adopt this new change. |
This PR is a patch for Modular TensorFlow Graph C API.
It removes global GrapplerItemMap. Optimizer API has changed from
void (*optimize_func)(void*, TF_Buffer*, TF_Buffer*, TF_Status*);tovoid (*optimize_func)(void*, const TF_Buffer*, const TF_GrapplerItem*, TF_Buffer*, TF_Status*);