Allow new `OpSchema`s to be initialized in Python #5020

justinchuby · 2023-03-21T04:15:58Z

Description

This PR needs to be in the 1.14 release

The change

Creates Python constructors for OpSchema, Attribute, FormalParameter and TypeConstraintParam.
Adds overloads for Schema::Input and Schema::Output that takes a FormalParameter
Updates the pyi file to reflect the change.
Hides the camel cased fields in FormalParameter for API consistency. snake_case versions are created and exposed, but this will not break existing usage as the camelCase versions are still retained, just not annotated in the pyi file. So new users will only discover the snake_case versions.

Code change

pybind update
Tests
Allow specifying default values in Attribute

Next PRs

Documentation
repr methods for the classes
OpSchema registration
Shape inference binding: create inference code with Python

Motivation and Context

#5019

justinchuby · 2023-03-22T00:54:30Z

cc @jcwchen @gramalingam

snapshot try this try this Fix compilation Signed-off-by: Justin Chu <justinchu@microsoft.com>

Signed-off-by: Justin Chu <justinchu@microsoft.com>

Fixed Signed-off-by: Justin Chu <justinchu@microsoft.com>

Signed-off-by: Justin Chu <justinchu@microsoft.com>

test Signed-off-by: Justin Chu <justinchu@microsoft.com>

Signed-off-by: Justin Chu <justinchu@microsoft.com>

onnx/test/schema_test.py

onnx/onnx_cpp2py_export/defs.pyi

Signed-off-by: Justin Chu <justinchu@microsoft.com>

onnx/test/schema_test.py

justinchuby · 2023-03-27T23:32:53Z

@jcwchen could you help add this to the milestone?

jcwchen · 2023-03-27T23:36:38Z

Added. Please note that the code freeze date is 4/7.

onnx/test/schema_test.py

justinchuby · 2023-04-04T03:15:37Z

@edgchen1 When we move the strings from the pybind apis, will python lose access to them? Should I not use a value after it was moved?

Edit: looks like they will be copied when the functions are being called?

Signed-off-by: Justin Chu <justinchu@microsoft.com>

justinchuby · 2023-04-04T16:37:35Z

I will follow up with documentation updates to keep this PR manageable. Also so that we can merge faster and iterate on wording in the doc

justinchuby · 2023-04-04T16:44:25Z

Also @gramalingam and @xadupre for review

onnx/cpp2py_export.cc

edgchen1 · 2023-04-04T21:38:04Z

When we move the strings from the pybind apis, will python lose access to them?
Looks like they will be copied when the functions are being called?

Since the string is passed by value, a string instance will be constructed (could be from a copy or a move from rvalue reference) for the argument. On the other hand, if it was passed by non-const reference, the caller might have to worry about it getting modified (e.g., moved from) even when the caller doesn't use std::move(x). I'm not sure what the pybind calling code looks like, possibly the string is copied there if Python still needs it.

Should I not use a value after it was moved?

Right. Generally, for types that support moving, moved from objects are in a valid but unspecified state. You could do limited things with such an object, e.g., assign to it or destroy it.

onnx/defs/schema.cc

onnx/cpp2py_export.cc

Signed-off-by: Justin Chu <justinchu@microsoft.com>

jcwchen

Basically LGTM. Since it's a large important feature, please get approvals from more reviewers before merge. Thanks for the effort!

Note: next items after this PR -- add corresponding API documentation in .md files and on web (onnx.ai).

onnx/cpp2py_export.cc

justinchuby · 2023-04-05T13:33:58Z

@xadupre for sig operator approval. Thanks!

Signed-off-by: Justin Chu <justinchu@microsoft.com>

onnx/defs/schema.cc

onnx/defs/schema.h

edgchen1

looks good!

Co-authored-by: Edward Chen <18449977+edgchen1@users.noreply.github.com> Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>

Signed-off-by: Justin Chu <justinchu@microsoft.com>

justinchuby requested a review from a team as a code owner March 21, 2023 04:15

justinchuby force-pushed the justinchu/py-schema branch from f988429 to 0e50421 Compare March 21, 2023 04:16

jcwchen added the utility label Mar 21, 2023

justinchuby force-pushed the justinchu/py-schema branch 2 times, most recently from 2fb734c to 197f5c9 Compare March 22, 2023 00:04

justinchuby added 5 commits March 22, 2023 04:08

snapshot

caf388e

snapshot try this try this Fix compilation Signed-off-by: Justin Chu <justinchu@microsoft.com>

Update

586159d

Signed-off-by: Justin Chu <justinchu@microsoft.com>

Try this

869a500

Fixed Signed-off-by: Justin Chu <justinchu@microsoft.com>

fix

0f08039

Signed-off-by: Justin Chu <justinchu@microsoft.com>

Update pyi

ab4c9aa

Signed-off-by: Justin Chu <justinchu@microsoft.com>

justinchuby force-pushed the justinchu/py-schema branch from 15acb03 to ca227c1 Compare March 22, 2023 04:08

Fix doc

60bf8c5

test Signed-off-by: Justin Chu <justinchu@microsoft.com>

justinchuby force-pushed the justinchu/py-schema branch from ca227c1 to 60bf8c5 Compare March 22, 2023 04:09

justinchuby and others added 2 commits March 21, 2023 21:14

Merge branch 'main' into justinchu/py-schema

5cb33d0

Tests

38bd1af

Signed-off-by: Justin Chu <justinchu@microsoft.com>

justinchuby force-pushed the justinchu/py-schema branch from 0aa9068 to 38bd1af Compare March 22, 2023 04:45

justinchuby commented Mar 22, 2023

View reviewed changes

onnx/test/schema_test.py Outdated Show resolved Hide resolved

github-advanced-security bot found potential problems Mar 22, 2023

View reviewed changes

onnx/test/schema_test.py Fixed Show fixed Hide fixed

justinchuby commented Mar 22, 2023

View reviewed changes

onnx/onnx_cpp2py_export/defs.pyi Outdated Show resolved Hide resolved

justinchuby and others added 2 commits March 22, 2023 16:22

More tests

ba85417

Signed-off-by: Justin Chu <justinchu@microsoft.com>

Merge branch 'main' into justinchu/py-schema

e74a63c

github-advanced-security bot found potential problems Mar 22, 2023

View reviewed changes

onnx/test/schema_test.py Fixed Show fixed Hide fixed

justinchuby mentioned this pull request Mar 24, 2023

OpSchema in Python microsoft/onnxscript#476

Closed

justinchuby marked this pull request as draft March 24, 2023 18:07

justinchuby changed the title ~~Make OpSchema writable~~ Allow new OpSchemas to be initialized in Python Mar 27, 2023

jcwchen added this to the 1.14.0 milestone Mar 27, 2023

github-advanced-security bot found potential problems Mar 30, 2023

View reviewed changes

onnx/test/schema_test.py Fixed Show fixed Hide fixed

onnx/test/schema_test.py Fixed Show fixed Hide fixed

onnx/test/schema_test.py Fixed Show fixed Hide fixed

justinchuby and others added 3 commits April 4, 2023 09:32

Add overloads

3e1a354

Signed-off-by: Justin Chu <justinchu@microsoft.com>

Merge branch 'main' into justinchu/py-schema

59c3e8e

Comment

28c3713

Signed-off-by: Justin Chu <justinchu@microsoft.com>

justinchuby requested review from jcwchen and edgchen1 April 4, 2023 16:42

xadupre reviewed Apr 4, 2023

View reviewed changes

onnx/cpp2py_export.cc Show resolved Hide resolved

edgchen1 reviewed Apr 4, 2023

View reviewed changes

onnx/defs/schema.cc Outdated Show resolved Hide resolved

onnx/defs/schema.cc Outdated Show resolved Hide resolved

onnx/cpp2py_export.cc Outdated Show resolved Hide resolved

onnx/cpp2py_export.cc Outdated Show resolved Hide resolved

onnx/cpp2py_export.cc Outdated Show resolved Hide resolved

justinchuby requested a review from xadupre April 4, 2023 23:40

justinchuby and others added 2 commits April 4, 2023 23:46

Let's get movin'

18b5c16

Signed-off-by: Justin Chu <justinchu@microsoft.com>

Merge branch 'main' into justinchu/py-schema

9872f63

justinchuby changed the title ~~Allow new OpSchemas to be initialized in Python~~ Allow new OpSchemas to be initialized in Python Apr 4, 2023

justinchuby mentioned this pull request Apr 5, 2023

feat(atenlib): add ops(avg pool2d) microsoft/onnxscript#559

Merged

jcwchen approved these changes Apr 5, 2023

View reviewed changes

onnx/cpp2py_export.cc Show resolved Hide resolved

Merge branch 'main' into justinchu/py-schema

bb695e7

Deprecation message

f542f11

Signed-off-by: Justin Chu <justinchu@microsoft.com>

edgchen1 reviewed Apr 5, 2023

View reviewed changes

onnx/defs/schema.cc Outdated Show resolved Hide resolved

edgchen1 reviewed Apr 5, 2023

View reviewed changes

onnx/defs/schema.h Outdated Show resolved Hide resolved

edgchen1 approved these changes Apr 5, 2023

View reviewed changes

justinchuby mentioned this pull request Apr 5, 2023

Auto generate OpSchema for functions microsoft/onnxscript#594

Closed

Update onnx/defs/schema.cc

cef5acd

Co-authored-by: Edward Chen <18449977+edgchen1@users.noreply.github.com> Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>

gramalingam approved these changes Apr 5, 2023

View reviewed changes

justinchuby and others added 2 commits April 5, 2023 18:17

Move declaration

102eb08

Signed-off-by: Justin Chu <justinchu@microsoft.com>

Merge branch 'main' into justinchu/py-schema

0b86f10

jcwchen merged commit ff6bd95 into onnx:main Apr 5, 2023
62 checks passed

justinchuby mentioned this pull request Apr 5, 2023

[Feature request] Make OpSchema writable from Python #5019

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow new `OpSchema`s to be initialized in Python #5020

Allow new `OpSchema`s to be initialized in Python #5020

justinchuby commented Mar 21, 2023 •

edited

justinchuby commented Mar 22, 2023

justinchuby commented Mar 27, 2023

jcwchen commented Mar 27, 2023

justinchuby commented Apr 4, 2023 •

edited

justinchuby commented Apr 4, 2023

justinchuby commented Apr 4, 2023

edgchen1 commented Apr 4, 2023

jcwchen left a comment

justinchuby commented Apr 5, 2023 •

edited

edgchen1 left a comment

Allow new OpSchemas to be initialized in Python #5020

Allow new OpSchemas to be initialized in Python #5020

Conversation

justinchuby commented Mar 21, 2023 • edited

Description

Motivation and Context

justinchuby commented Mar 22, 2023

justinchuby commented Mar 27, 2023

jcwchen commented Mar 27, 2023

justinchuby commented Apr 4, 2023 • edited

justinchuby commented Apr 4, 2023

justinchuby commented Apr 4, 2023

edgchen1 commented Apr 4, 2023

jcwchen left a comment

Choose a reason for hiding this comment

justinchuby commented Apr 5, 2023 • edited

edgchen1 left a comment

Choose a reason for hiding this comment

Allow new `OpSchema`s to be initialized in Python #5020

Allow new `OpSchema`s to be initialized in Python #5020

justinchuby commented Mar 21, 2023 •

edited

justinchuby commented Apr 4, 2023 •

edited

justinchuby commented Apr 5, 2023 •

edited