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

chore(gpu): remove lwe chunk size argument from the multi-bit PBS #1445

Merged
merged 1 commit into from
Aug 2, 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 Aug 1, 2024
@agnesLeroy agnesLeroy requested a review from pdroalves August 1, 2024 09:57
@agnesLeroy
Copy link
Contributor Author

I think we may have had some mismatch with this lwe_chunk_size argument, and maybe it was causing the unspecified launch failure. I'm running the H100 tests several times to check it.

@agnesLeroy agnesLeroy force-pushed the al/fix_lwe_chunkk_size_arg branch from 9a2f614 to cc4721a Compare August 1, 2024 10:12
@agnesLeroy
Copy link
Contributor Author

agnesLeroy commented Aug 1, 2024

Now I'm wondering if we want to keep this argument, we're not using it actually... I've force pushed to the branch to remove it altogether.

@agnesLeroy agnesLeroy force-pushed the al/fix_lwe_chunkk_size_arg branch from cc4721a to 2fe63c9 Compare August 1, 2024 11:32
@agnesLeroy agnesLeroy changed the title chore(gpu): fix lwe chunk size argument chore(gpu): remove lwe chunk size argument from the multi-bit PBS Aug 1, 2024
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.

That argument is indeed not needed anymore.

To remove it you also need to update the function that generates the arguments for the internal benchmark.

@pdroalves pdroalves self-requested a review August 1, 2024 11:48
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.

Ops, there is actually something to change before approval. Sorry!

@agnesLeroy agnesLeroy force-pushed the al/fix_lwe_chunkk_size_arg branch from 2fe63c9 to 91bfbef Compare August 1, 2024 11:51
@zama-bot zama-bot removed the approved label Aug 1, 2024
@agnesLeroy
Copy link
Contributor Author

agnesLeroy commented Aug 1, 2024

I just made the change 🙂 - I triggered approval CI again via the label

@agnesLeroy agnesLeroy closed this Aug 1, 2024
@agnesLeroy agnesLeroy reopened this Aug 1, 2024
@zama-bot zama-bot removed the approved label Aug 1, 2024
@pdroalves pdroalves self-requested a review August 1, 2024 20:22
@agnesLeroy agnesLeroy merged commit 7fa9f33 into main Aug 2, 2024
140 of 149 checks passed
@agnesLeroy agnesLeroy deleted the al/fix_lwe_chunkk_size_arg branch August 2, 2024 07:12
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.

3 participants