Skip to content

Add support for parsing simple brevitas layers as part of pytorch models #1019

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

Open
wants to merge 71 commits into
base: main
Choose a base branch
from

Conversation

JanFSchulte
Copy link
Contributor

This PR adds support for the parsing of simple brevitas layers (QuantLinear, QuantActivation, QuantConv1D, QuantConv2D) to the pytorch parser. More complex models will still have to go through QONNX, but simple cases can be handled easily within the pytorch parser itself. To this end, this PR adds a new quantizer which only propagates the desired precision to the hsl4ml model, as brevitas already provides the quantized tensors which we pick up directly.

Type of change

For a new feature or function, please create an issue first to discuss it
with us before submitting a pull request.

Note: Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Tests

Tested locally with simple model and added pytests.

Checklist

  • I have read the guidelines for contributing.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • I have installed and run pre-commit on the files I edited or added.
  • I have added tests that prove my fix is effective or that my feature works.

@JanFSchulte
Copy link
Contributor Author

pre-commit.ci autofix

@JanFSchulte JanFSchulte added the please test Trigger testing by creating local PR branch label Jun 7, 2024
@jmitrevs
Copy link
Contributor

Do we need to add brevitas to our test environment docker image? Or can we just add brevitas as a testing dependency? I think that's why the test failed.

@jmitrevs jmitrevs added please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Jul 9, 2024
@JanFSchulte
Copy link
Contributor Author

pre-commit.ci autofix

@JanFSchulte JanFSchulte added please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Mar 21, 2025
@JanFSchulte JanFSchulte added please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Mar 26, 2025
@vloncar vloncar mentioned this pull request Mar 26, 2025
7 tasks
@JanFSchulte
Copy link
Contributor Author

Hey @jmitrevs I can't reproduce the oneAPI compilation error in the test_brevitas_parsing pytest locally for some reason. I'm suspecting it's genuine, but it's hard to tell because locally it compiles fine. I have oneAPI version 2025.0 installed. Could you see if you can reproduce this?

@JanFSchulte
Copy link
Contributor Author

To summarize the current state of this: I think this PR contains what we can reasonably support currently and most things work somewhat well. Everything is limited to power-of-2 scales,but I think that's fine for now. The big issue that I still have is QuantLSTM. Brevitas completely reimplements RNNs and re-quantizes frequently internally. I tried to match that by setting the precision of the internally used variables more granular than we currently do, but the precision is horrible. I am tempted to remove it for the moment. Currently, you can't even export QuantLSTM to QONNX, so this is something that brevitas itself is still figuring out.

@JanFSchulte JanFSchulte added please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Apr 2, 2025
@bo3z bo3z modified the milestones: v1.1.0, v1.2.0 Apr 8, 2025
@JanFSchulte
Copy link
Contributor Author

Removed QuantLSTM for now. QuantRNN is also not great, but at least usually gets results in the same ballpark. Otherwise I think this is ready for review.

@JanFSchulte JanFSchulte added please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Apr 9, 2025
@vloncar vloncar self-requested a review April 9, 2025 17:39
@JanFSchulte JanFSchulte added please test Trigger testing by creating local PR branch and removed please test Trigger testing by creating local PR branch labels Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please test Trigger testing by creating local PR branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants