tensorflow / community

Stores documents used by the TensorFlow developer community
Apache License 2.0
1.26k stars 579 forks source link

RFC: Kernel C API extension for Variable ops. #387

Closed kulinseth closed 3 years ago

kulinseth commented 3 years ago

The RFC will be open for comment until Tuesday, May 18, 2021

Status Proposed
RFC # 387
Author(s) Kulin Seth (Apple), Charles Brissart (Apple)
Sponsor Saurabh Saksena (srbs@google.com)
Updated 2021-05-04

The proposal extends the current Kernel C API to enable plugin writers using Modular API to add support for :

penpornk commented 3 years ago

If you are interested in attending the design review meeting for this RFC, here are the call details:

Time: Tuesday, May 18th, 2021 from 10:00-11:00am PT Meeting link: Google Meet

Meeting participants are expected to read the RFC ahead of time as the meeting will focus on the remaining issues/questions.

ematejska commented 3 years ago

This was accepted at the design review meeting today.

penpornk commented 3 years ago

Design review meeting notes

@kulinseth: We started by exposing each helper function for optimizer / Resource variables through C API. Based on @saxenasaurabh's comments, GetInputTensorFromVariable can be used for Resource variables too. We'll refine them in the actual implementation PR.

@saxenasaurabh: How do you scope the set of ops? Do you have a fallback mechanism? @kulinseth: We looked through 14-15 networks and dumped out all the ops. For TensorArray, we fall back to CPU.

@kulinseth: @jzhoulon, what models do you need TensorArray and UnbatchResource for? @jzhoulon / @Jianhui-Li: No specific models. Just for ops coverage / completeness. We looked at TensorArray, Variables, ResourceMgr, BatchResource-related ops. The API you exposed is cleaner.

@kulinseth: Are ResourceManager and TensorArray needed?

@kulinseth: In the PR, we will break up input tensor from Variable.

@kulinseth: We recently added a helper function TF_GetInputByName to the RFC.

@Jianhui-Li: TensorArray is replaced by TensorList* in v2. What about UnbatchResource?

@kulinseth: We can send out a PR today so everybody can take a closer look.

kulinseth commented 3 years ago

Thanks @penpornk for detailed notes. I will update here with PR.

penpornk commented 3 years ago

Here is @kulinseth's implementation PR: https://github.com/tensorflow/tensorflow/pull/49717