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

[BYOC] Remove ext params stored in metadata from params to avoid duplication #7977

Merged
merged 2 commits into from
May 6, 2021

Conversation

trevor-m
Copy link
Contributor

@trevor-m trevor-m commented May 4, 2021

This PR is an alternative to #7564

During compilation, weights from external modules are stored in the MetadataModule (.so), but they are still kept in params so they end up being duplicated.

This PR will remove from params only the weights which were stored in MetadataModule, solving the weight duplication issue.

@trevor-m
Copy link
Contributor Author

trevor-m commented May 4, 2021

Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

The change looks clean and good to me. Could you add a test case?

@trevor-m
Copy link
Contributor Author

trevor-m commented May 5, 2021

Thanks @comaniac
I added an assert to an existing test.
Without this pr, the assert fails:

        # Params will be stored in metadata module.
>       assert len(graph_module.get_params()) == 0
E       AssertionError: assert 1 == 0
E        +  where 1 = len({'ccompiler_0_const_0': <tvm.nd.NDArray shape=(1,), cpu(0)>\narray([1.], dtype=float32)})
E        +    where {'ccompiler_0_const_0': <tvm.nd.NDArray shape=(1,), cpu(0)>\narray([1.], dtype=float32)} = <bound method GraphExecutorFactoryModule.get_params of <tvm.relay.backend.graph_executor_factory.GraphExecutorFactoryModule object at 0x7fd7e2831dd8>>()
E        +      where <bound method GraphExecutorFactoryModule.get_params of <tvm.relay.backend.graph_executor_factory.GraphExecutorFactoryModule object at 0x7fd7e2831dd8>> = <tvm.relay.backend.graph_executor_factory.GraphExecutorFactoryModule object at 0x7fd7e2831dd8>.get_params

Copy link
Contributor

@comaniac comaniac left a comment

Choose a reason for hiding this comment

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

LGTM

@comaniac comaniac merged commit 5a2ded7 into apache:main May 6, 2021
@comaniac
Copy link
Contributor

comaniac commented May 6, 2021

Thanks @trevor-m

trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
…ication (apache#7977)

* Remove ext params stored in metadata from params to avoid duplication

* Add test for duplicate params
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
…ication (apache#7977)

* Remove ext params stored in metadata from params to avoid duplication

* Add test for duplicate params
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request May 6, 2021
…ication (apache#7977)

* Remove ext params stored in metadata from params to avoid duplication

* Add test for duplicate params
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request May 11, 2021
…ication (apache#7977)

* Remove ext params stored in metadata from params to avoid duplication

* Add test for duplicate params
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.

2 participants