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

Feat: remove quant metadata from quantlayer #883

Merged
merged 3 commits into from
Mar 28, 2024

Conversation

Giuseppe5
Copy link
Collaborator

@Giuseppe5 Giuseppe5 commented Feb 27, 2024

Quant layer should not expose any attribute of their underlying proxys (e.g, quant_output_scale in QuantLayer).

Similarly, caching should be delegated to Proxy.

Missing tasks:

  • Remove comments
  • Armonize interface (is_signed vs signed())
  • Move caching all the way down to proxy (including flags to enable/disable caching)

As task for another PR, we should programmatically generate all the methods that were removed in this PR, such as layer.weight_quant_scale(). The Proxy should expose which methods the layer can pick up, and then generate that using proxy suffix + name of the method.

@Giuseppe5 Giuseppe5 self-assigned this Feb 27, 2024
@Giuseppe5 Giuseppe5 force-pushed the move_caching branch 2 times, most recently from 6077380 to 8849800 Compare March 6, 2024 22:28
@Giuseppe5 Giuseppe5 requested review from nickfraser and removed request for nickfraser March 7, 2024 12:03
@Giuseppe5 Giuseppe5 force-pushed the move_caching branch 2 times, most recently from bbf4cc4 to 95df101 Compare March 14, 2024 00:37
Copy link
Collaborator

@costigt-dev costigt-dev left a comment

Choose a reason for hiding this comment

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

looks good, the changes like quant_weight_scale to quant_weight.scale are nice

@Giuseppe5 Giuseppe5 requested review from nickfraser and removed request for nickfraser March 27, 2024 07:51
@Giuseppe5 Giuseppe5 requested review from nickfraser and removed request for nickfraser March 27, 2024 08:52
Copy link
Collaborator

@nickfraser nickfraser left a comment

Choose a reason for hiding this comment

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

LGTM!

@Giuseppe5 Giuseppe5 merged commit e1da07b into Xilinx:dev Mar 28, 2024
346 of 347 checks passed
@Giuseppe5 Giuseppe5 deleted the move_caching branch March 28, 2024 12:29
Giuseppe5 added a commit to Giuseppe5/brevitas that referenced this pull request Apr 12, 2024
Breaking change: The interface to access quant metadata has changed and now everything is directly delegated to the underlying proxies.
Giuseppe5 added a commit to Giuseppe5/brevitas that referenced this pull request Apr 12, 2024
Breaking change: The interface to access quant metadata has changed and now everything is directly delegated to the underlying proxies.
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