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

Overflow detection in simulation #777

Merged
merged 5 commits into from
May 15, 2024
Merged

Conversation

youben11
Copy link
Member

@youben11 youben11 commented Apr 4, 2024

No description provided.

@cla-bot cla-bot bot added the cla-signed label Apr 4, 2024
@youben11 youben11 force-pushed the feat/detect-overflow-simulation branch from dc1c6c7 to 5abf580 Compare April 4, 2024 11:21
Copy link
Member

@BourgerieQuentin BourgerieQuentin left a comment

Choose a reason for hiding this comment

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

As well should have tests

Copy link
Contributor

@umut-sahin umut-sahin left a comment

Choose a reason for hiding this comment

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

Also, we need to check for errors after table lookups as they can result in overflows (imagine, [1, 2, 3000, 4] table and 2 is not observed in evaluation, but happened in simulation)

@youben11 youben11 force-pushed the feat/detect-overflow-simulation branch 5 times, most recently from e0a0ee9 to 32f0998 Compare April 9, 2024 15:20
Copy link
Member

@BourgerieQuentin BourgerieQuentin left a comment

Choose a reason for hiding this comment

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

On my previous review I totally forgot the signed integer case, actually the encoding differs from unsigned and signed integers. On unsigned integers we let the padding bit clean, but on signed integers we use the padding bit as the traditional sign bit to let the leveled signed operations works natively then we rescale before tlu.
So we should check overflow in different way on signed (against 64bits) and unsigned (against 63bits). One of the issue is that the signed information is not available at TFHE level. Let's live discuss of that.

@youben11 youben11 force-pushed the feat/detect-overflow-simulation branch 3 times, most recently from ea25a58 to 792e5cf Compare April 24, 2024 16:23
@youben11 youben11 force-pushed the feat/detect-overflow-simulation branch 3 times, most recently from 3c4f5e2 to c2ec1a4 Compare April 30, 2024 17:49
@youben11 youben11 requested a review from BourgerieQuentin May 14, 2024 14:36
@youben11 youben11 force-pushed the feat/detect-overflow-simulation branch from 0ea44f2 to 463a1b3 Compare May 15, 2024 07:51
@youben11 youben11 merged commit b6a43cf into main May 15, 2024
31 checks passed
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