-
-
Notifications
You must be signed in to change notification settings - Fork 487
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
Fixed and improvements in is_LLL_reduced
and approximate_closest_vector
#38259
Conversation
Documentation preview for this PR (built with commit 605c02a; changes) is ready! 🎉 |
Lint failures |
The code coverage that's missing feels a bit silly, should I still create tests for them? (one of them is unreachable so idk) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small comment about comment formatting, otherwise good
This fixes #38115. Along with the goal stated there I have fixed some stuff I wasn't happy with from the first iteration of
approximate_closest_vector
. These changes could be breaking but seeing as there has been no release since its introduction this shouldn't be a problem.Changes:
algorithm
parameter tois_LLL_reduced
along with the new algorithmfpLLL
which uses fpylll'sLLL.is_reduced
method, and made it the default, as I see no advantages with the old implementation.algorithm
parameter toapproximate_closest_vector
along with the new algorithmsembedding
androunding_off
, and madeembedding
the default.Changed the alias ofapproximate_closest_vector
frombabai
tocvp
to be more accurate but still short.I'm getting weird errors when building documentation locally but I don't think it's related to my changes, so I haven't been able to check if the doc build looks alright yet. (and it won't build on my own repo b/c my GitHub username has uppercase letters in it :)
📝 Checklist