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

Model-local functions when creating a ModelProto from a python function #59

Open
gramalingam opened this issue Jun 6, 2022 · 3 comments
Labels
enhancement New feature or request topic: api

Comments

@gramalingam
Copy link
Collaborator

When we export/convert a Python function to a ModelProto, we need to identify the set of functions that will be included in the generated ModelProto as model-local functions. Furthermore, the users should be able to control this. For example, I might want to create model that calls Relu, with or without including the function-definition for Relu (even though we might have a function-definition for Relu available).

(See PR: #41 )

@gramalingam
Copy link
Collaborator Author

Another related feature is that of allowing potentially multiple function-definitions for the same op (in a code-base). It would be good if the underlying design enables this. It is also related to the notion of a "library" (a "LibProto" in serialized form) to represent a set of function-definitions bundled together. And to the ability to "link" a ModelProto against one or more libraries.

@gramalingam
Copy link
Collaborator Author

gramalingam commented Jun 15, 2022

I suggest that we remove the embedded functions stored in the IR objects (for Stmt and Function), and instead make to_model_proto and to_graph_proto take a resolver object as a parameter, where a resolver is a dictionary-like object that, given a function-identifier (name, opset, version) returns a FunctionProto or None. The goal is to decouple the mechanism used to maintain a mapping from function-identifiers to their corresponding FunctionProto instead of hard-coding it into the IR itself. This will help address the key concerns, such as below:

The behavior of foo.to_model_proto() should be well-defined. As it exists today, assume that foo calls bar. If we do an import bar_def that has the effect of defining the body of bar, then foo_to_model_proto() will include the definition of bar as a model-local function, otherwise it won't. So, the behavior of to_model_proto ends up depending on hidden-state. It is better if the behavior is explicitly captured by an extra-parameter resolver of to_model_proto.

@justinchuby
Copy link
Contributor

This would be nice 👍

@justinchuby justinchuby added enhancement New feature or request topic: api labels Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request topic: api
Projects
None yet
Development

No branches or pull requests

2 participants