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

bugfix(prob) handle sigma > q #80

Merged
merged 1 commit into from
Aug 23, 2023
Merged

bugfix(prob) handle sigma > q #80

merged 1 commit into from
Aug 23, 2023

Conversation

bencrts
Copy link
Collaborator

@bencrts bencrts commented Aug 22, 2023

Leaving in draft for now, will try to come up with a better condition than sigma > q as it's not the one we want.

@malb
Copy link
Owner

malb commented Aug 22, 2023

I don't think this is the correct fix. IIRC the KF analysis of BKW on Binary LWE uses sigma > q. The advantage is small, but not zero, e.g.

sage: q=2^13; sigma=2^15; log(float(exp(-float(pi) * (float(sigma / q) ** 2))), 2.)
-72.5177622692351

@bencrts
Copy link
Collaborator Author

bencrts commented Aug 22, 2023

Indeed I agree -- I've left the PR as draft for now. I'll take a look at the advantage formula and edit to someting more reasonable asap.

@bencrts
Copy link
Collaborator Author

bencrts commented Aug 22, 2023

Alternatively we could just use some larger arbitrary bound, the problem was that the optimization was getting to:

sage: sigma = 2.32635228499049e947
sage: q = 16369619120181469452283419146795337838756316829754574698247620392195385261672706601332322013852753
....: 903223620612005347547568443576301324009744462831784432007180117250379339731260566921104165956679636026
....: 071068887222235733068105836856857083444590817350909528504074694291691092139548999928924484582086252478
....: 977633818457376405409231086116246168367047039176965052754559121441305592658834773042270186227068683230
....: 815767812692570326671423805343812399828024592832603292760006139062134619510217374480970637509621909629
....: 132424341126237483954993915411214076712692111051902940729020471900580101114968926454825939983166452660
....: 569882221948169114307695627932888903160524898910557246807652042223153662188221235158821666169210876076
....: 693138615217053841090670627933973595298962019709647311664185065483481987899214266368
sage: sigma/q
1.42114014254762e154

@malb
Copy link
Owner

malb commented Aug 23, 2023

Yeah, the advantage log base 2 is RR(-pi*gap^2)/log(2.,e), so e.g. sigma/q > 16 should cover us

@bencrts
Copy link
Collaborator Author

bencrts commented Aug 23, 2023

ah nice, thanks! Made the change and looks like all tests pass -- will take the PR out of draft.

@bencrts bencrts marked this pull request as ready for review August 23, 2023 10:01
@malb malb merged commit 8f85aa2 into main Aug 23, 2023
4 checks passed
@bencrts bencrts deleted the issue-78 branch August 23, 2023 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants