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

fix: propagate GLWE config on hybrid MLP unit test #928

Conversation

alinealfa
Copy link
Contributor

Notes

Fix #927 - Address test_hybrid_converter.py failure on MacOS

This fix propagates the GLWE configuration in the hybrid MLP unit test.
Review needed to confirm if this aligns with the test's original intended behavior.

Testing

Executed the make tasks as outlined in contributing.md. For pytest, used pytest_no_flaky due to a local unrelated test_brevitas_qat.py test failure, which is known to be flaky.

@alinealfa alinealfa requested a review from a team as a code owner October 25, 2024 13:29
Copy link

cla-bot bot commented Oct 25, 2024

Thank you for your pull request. We require contributors to sign our Contributor License Agreement / Terms and Conditions, and we don't seem to have the users @alinealfa on file. In order for us to review and merge your code, please sign:

  • For individual contribution: our CLA
  • for Bounty submission, if you are an individual: our T&C
  • for Bounty submission, if you are a company: our T&C
    to get yourself added.

If you already signed one of this document, just wait to be added to the bot config.

@aquint-zama
Copy link
Contributor

@cla-bot check

@cla-bot cla-bot bot added the cla-signed label Oct 25, 2024
Copy link

cla-bot bot commented Oct 25, 2024

The cla-bot has been summoned, and re-checked this pull request!

Copy link
Collaborator

@andrei-stoian-zama andrei-stoian-zama left a comment

Choose a reason for hiding this comment

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

Thanks for the refactoring here!

@andrei-stoian-zama andrei-stoian-zama self-requested a review October 28, 2024 10:33
@alinealfa alinealfa force-pushed the fix/927_propagate_glwe_config_on_test_hybrid_mlp branch from e27c7db to b0133da Compare October 28, 2024 11:58
@alinealfa
Copy link
Contributor Author

alinealfa commented Oct 28, 2024

Thanks @andrei-stoian-zama. I updated the commit as the CI was enforcing signature.

@andrei-stoian-zama
Copy link
Collaborator

@alinealfa , as we require signed commits only from approved Zama employees, I needed to re-base your PR . It's here: #935 and I'll close this one.

@alinealfa
Copy link
Contributor Author

@alinealfa , as we require signed commits only from approved Zama employees, I needed to re-base your PR . It's here: #935 and I'll close this one.

Got it. So should external contributors not sign the commit? Actually the Contributing doc does not indicate need of signature. I ended up adding so we could get the CI/tests unblocked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test test_hybrid_converter.py failing on MacOS
3 participants