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

[Relax] Add CopyWithNewParams interface #91

Merged

Conversation

Ubospica
Copy link
Contributor

@Ubospica Ubospica commented Jan 9, 2023

This is the new PR following #90 after the source branch having been moved to personal repositiory.

This PR adds a new interface relax.utils.copy_with_new_params to copy functions:

def copy_with_new_params(func: Function) -> Function:
    pass

The parameters of the original function would be copied to satisfy the restriction in the well-formed check: any two functions should not share the same parameter variable.

This interface serves for passes that needs to copy some existing function and modify it, such as relax.transform.Gradient.

@Ubospica Ubospica force-pushed the mlc-dev/2023-01-07-transform-copyfunc-mine branch from 2f87224 to e56e2de Compare January 9, 2023 05:48
@Ubospica Ubospica changed the title [Relax] Add CopyWithNewParam interface [Relax] Add CopyWithNewParams interface Jan 9, 2023
@SiriusNEO
Copy link
Contributor

I'm thinking if the struct info of params contains Var, should we copy them as well?
For example, the shape of an input Tensor is a relax.Var.

@Ubospica
Copy link
Contributor Author

Ubospica commented Jan 9, 2023

I'm thinking if the struct info of params contains Var, should we copy them as well? For example, the shape of an input Tensor is a relax.Var.

@SiriusNEO I perfer not. Because we now only have to satisfy the requirement in well-formed check, which only checks parameter vars of functions. If we wanna copy all vars in the function, we may have to do this.

src/relax/utils.cc Outdated Show resolved Hide resolved
@MasterJH5574
Copy link
Member

I'm thinking if the struct info of params contains Var, should we copy them as well?
For example, the shape of an input Tensor is a relax.Var.

The behavior and semantics of param struct info containing shape var needs more thinking, and currently we have no such example. We will revisit this point after some time and at this moment let’s just go as usual, assuming no shape var appears in the params’ structure info.

@Ubospica Ubospica force-pushed the mlc-dev/2023-01-07-transform-copyfunc-mine branch from a30c2a7 to b5bf1ee Compare January 10, 2023 03:45
@Ubospica Ubospica changed the base branch from structinfo to relax January 10, 2023 03:46
@MasterJH5574 MasterJH5574 merged commit ca8553d into mlc-ai:relax Jan 10, 2023
MasterJH5574 pushed a commit that referenced this pull request Jan 12, 2023
* copy_func pass

* reformat

* move interface into utils

* simplify test

* Change name to copy_with_new_params

* remove zero-param constructor

* change visitor into Transform

* formatted

* finish revise

* revise
@Ubospica Ubospica deleted the mlc-dev/2023-01-07-transform-copyfunc-mine branch January 13, 2023 02:10
MasterJH5574 pushed a commit that referenced this pull request Jan 16, 2023
Co-authored-by: Prakalp Srivastava <prakalp@octoml.ai>
MasterJH5574 pushed a commit that referenced this pull request Jan 16, 2023
* copy_func pass

* reformat

* move interface into utils

* simplify test

* Change name to copy_with_new_params

* remove zero-param constructor

* change visitor into Transform

* formatted

* finish revise

* revise
MasterJH5574 pushed a commit that referenced this pull request Jan 28, 2023
* copy_func pass

* reformat

* move interface into utils

* simplify test

* Change name to copy_with_new_params

* remove zero-param constructor

* change visitor into Transform

* formatted

* finish revise

* revise
Ubospica added a commit to Ubospica/relax-develop that referenced this pull request Jan 29, 2023
* copy_func pass

* reformat

* move interface into utils

* simplify test

* Change name to copy_with_new_params

* remove zero-param constructor

* change visitor into Transform

* formatted

* finish revise

* revise
MasterJH5574 pushed a commit that referenced this pull request Feb 8, 2023
Co-authored-by: Prakalp Srivastava <prakalp@octoml.ai>
spectrometerHBH pushed a commit to spectrometerHBH/relax that referenced this pull request Feb 9, 2023
Co-authored-by: Prakalp Srivastava <prakalp@octoml.ai>
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.

3 participants