Skip to content

Conversation

@rafzi
Copy link
Contributor

@rafzi rafzi commented Jun 24, 2022

Copy link
Contributor

@mbs-octoml mbs-octoml left a comment

Choose a reason for hiding this comment

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

LGTM, thanks and sorry for the glitch. Just some nits to make the API more convenient as per the c++ API.

fn,
list(new_params),
new_body,
fn.ret_type,
Copy link
Contributor

Choose a reason for hiding this comment

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

You can elide all the fn.* args since WithFields is designed to make it easy to preserve everything that's unchanged from the original expression.



@tvm._ffi.register_func("relay.FunctionWithFields")
def FunctionWithFields(function, params, body, ret_type, ty_params, attrs, virtual_device, span):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think defaulting all params after function to None will mimic the c++ version and support the above suggestion.

return Function(params, body, ret_type, ty_params, attrs);
});
TVM_REGISTER_GLOBAL("relay.ir.FunctionWithFields")
.set_body_typed([](Function function, tvm::Array<Var> params, Expr body, Type ret_type,
Copy link
Contributor

Choose a reason for hiding this comment

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

Preserve the Optional<...> on all params after function to support the above suggestions.

@masahi
Copy link
Member

masahi commented Jun 27, 2022

@tvm-bot rerun

@rafzi rafzi force-pushed the patch-funcwithfields branch from 1921f44 to ad5dcd7 Compare June 28, 2022 08:56
@masahi masahi merged commit 6c433d2 into apache:main Jun 28, 2022
blackkker pushed a commit to blackkker/tvm that referenced this pull request Jul 7, 2022
…in ExprMutator (apache#11882)

* [Relay][VirtualDevice] Expose WithFields to Python to do proper copy in ExprMutator

* [Relay] give FunctionWithFields optional arguments

* [lint] fix wrong line length

* [lint] missing newline

* [doc] add doc string to FunctionWithFields
masahi pushed a commit to masahi/tvm that referenced this pull request Jul 15, 2022
…in ExprMutator (apache#11882)

* [Relay][VirtualDevice] Expose WithFields to Python to do proper copy in ExprMutator

* [Relay] give FunctionWithFields optional arguments

* [lint] fix wrong line length

* [lint] missing newline

* [doc] add doc string to FunctionWithFields
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
…in ExprMutator (apache#11882)

* [Relay][VirtualDevice] Expose WithFields to Python to do proper copy in ExprMutator

* [Relay] give FunctionWithFields optional arguments

* [lint] fix wrong line length

* [lint] missing newline

* [doc] add doc string to FunctionWithFields
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants