-
Notifications
You must be signed in to change notification settings - Fork 133
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
docs(frontend): adding a use-case for Levenshtein distance #902
base: main
Are you sure you want to change the base?
Conversation
f61fa55
to
122de8c
Compare
Works quite well but fail from time to time:
I have |
c9b3afb
to
d6720e5
Compare
Now:
|
Hey @umut-sahin , would you have a look, to see if we can make it faster, please? |
frontends/concrete-python/examples/levenshtein_distance/levenshtein_distance.py
Show resolved
Hide resolved
frontends/concrete-python/examples/levenshtein_distance/levenshtein_distance.py
Outdated
Show resolved
Hide resolved
frontends/concrete-python/examples/levenshtein_distance/levenshtein_distance.py
Outdated
Show resolved
Hide resolved
frontends/concrete-python/examples/levenshtein_distance/levenshtein_distance.py
Outdated
Show resolved
Hide resolved
0ee22ef
to
e2dceee
Compare
Now I have added a system for alphabet: eg, one can do
and you'll see it generates random string of A, C, T and G. |
It's very easy to add a new alphabet |
And there is a new option, to make perks on all alphabets:
|
What do you think guys? When you like the code, I add a small .md and measure that on AWS |
63c858e
to
1247eda
Compare
1247eda
to
2771fed
Compare
No more blocked by 794 |
frontends/concrete-python/examples/levenshtein_distance/levenshtein_distance.py
Outdated
Show resolved
Hide resolved
frontends/concrete-python/examples/levenshtein_distance/levenshtein_distance.md
Show resolved
Hide resolved
print("") | ||
|
||
if args.autoperf: | ||
for alphabet in ["ACTG", "string", "STRING", "StRiNg"]: |
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.
I'd really prefer to have a class for the alphabet with encode
, decode
, check
, ... methods. It'd make the example much clear IMO.
Also you would be able to do:
for alphabet in [Alphabet.lowercase(), Alphabet.uppercase(), Alphabet.dna(), ...]:
Which is less error prone and more clear IMO.
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.
I'm not a big fan of OOP, especially when like here, it doesn't seem needed. But if you want really it, I'll do. (And for sure, I have not the right habits for OOP so I am going to make something not idomatic, you'll correct me)
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.
I don't think this is OOP. Sure it has a bit of encapsulation and maybe some abstraction, but there is no inheritance or polymorphism. It's just a way to make alphabets more type safe and understandable.
Would be better to me, wdyt @BourgerieQuentin?
frontends/concrete-python/examples/levenshtein_distance/levenshtein_distance.md
Show resolved
Hide resolved
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.
Minor comment feel free to include or not, thanks.
Adding a use-case for Levenshtein distance
closes #https://github.com/zama-ai/concrete-internal/issues/750