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

Implement GPU signed scalar div, add scalar div to the hl api #1529

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

agnesLeroy
Copy link
Contributor

closes: please link all relevant issues

PR content/description

Check-list:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Relevant issues are marked as resolved/closed, related issues are linked in the description
  • Check for breaking changes (including serialization changes) and add them to commit message following the conventional commit specification

@cla-bot cla-bot bot added the cla-signed label Sep 11, 2024
@agnesLeroy agnesLeroy marked this pull request as ready for review September 11, 2024 15:31
@agnesLeroy agnesLeroy force-pushed the al/signed_scalar_div branch 5 times, most recently from b5d50ce to 4e8bdc4 Compare September 12, 2024 12:02
@zama-bot zama-bot removed the approved label Sep 13, 2024
@agnesLeroy agnesLeroy force-pushed the al/signed_scalar_div branch 4 times, most recently from 58e25e1 to ec45bcc Compare September 16, 2024 12:20
Copy link
Contributor

@pdroalves pdroalves left a comment

Choose a reason for hiding this comment

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

Hey Agnes!

I was not able to review the HL API. Regarding the integer part, I missed the assign_async/async logic we always use for those operators.

@agnesLeroy
Copy link
Contributor Author

@pdroalves I modified the PR, I changed also the cast functions to have sync/async variants. The PR is significantly bigger now but I think this improves things a lot, thank you very much for your review.

@tmontaigu
Copy link
Contributor

The hlapi part looks good

Copy link
Contributor

@pdroalves pdroalves left a comment

Choose a reason for hiding this comment

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

Seems good! Just added two minor comments. There is a forgotten unsafe clause.

@agnesLeroy agnesLeroy merged commit 24088fd into main Sep 19, 2024
80 of 83 checks passed
@agnesLeroy agnesLeroy deleted the al/signed_scalar_div branch September 19, 2024 17:11
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.

4 participants