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

FHE Regex Pattern Matching Engine #278

Merged
merged 2 commits into from
May 24, 2023
Merged

FHE Regex Pattern Matching Engine #278

merged 2 commits into from
May 24, 2023

Conversation

RKlompUU
Copy link
Contributor

@RKlompUU RKlompUU commented May 7, 2023

Most important locations:

  • the tutorial is in tfhe/docs/regex/tutorial.md
  • the example implementation is in tfhe/src/regex/..

Not quite sure if you want the example implementation to be under tfhe/src directly (how it is in this PR atm), or to have it be more of an external dependency (like how I initially had set it up in a separate repo). Let me know.

I might still want to add a Further work section at the end of the tutorial, and to add some comments to the code. But I think it's best to open this PR now to get the final review process started.


To run the included tests run: cargo test --release --features=x86_64,integer --example regex_engine.

@IceTDrinker
Copy link
Member

first request for this PR: squash in a single commit and follow the commit message format, in that case it should be

docs(tfhe): add FHE Regex Pattern Matching Engine

(... more details)

Copy link
Contributor

@tmontaigu tmontaigu left a comment

Choose a reason for hiding this comment

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

I still need to review a bit more deeply I looked good from what i've seen so far, I've made some comments.

Not quite sure if you want the example implementation to be under tfhe/src directly (how it is in this PR atm), or to have it be more of an external dependency (like how I initially had set it up in a separate repo). Let me know.

Yes we would prefer the implementation to be more separated from the lib.
Normally you should be able to have all your code under a sub folder of the examples folder
You could take at look at this other bounty where its organized in the example folder #283

tfhe/docs/regex/tutorial.md Outdated Show resolved Hide resolved
tfhe/docs/regex/tutorial.md Outdated Show resolved Hide resolved
tfhe/docs/regex/tutorial.md Outdated Show resolved Hide resolved
tfhe/docs/regex/tutorial.md Outdated Show resolved Hide resolved
tfhe/src/regex/ciphertext.rs Outdated Show resolved Hide resolved
tfhe/Cargo.toml Outdated Show resolved Hide resolved
tfhe/test_data/client_key Outdated Show resolved Hide resolved
tfhe/Cargo.toml Outdated Show resolved Hide resolved
@RKlompUU RKlompUU force-pushed the main branch 4 times, most recently from 57bbbf3 to f7ecbc2 Compare May 11, 2023 20:25
@RKlompUU
Copy link
Contributor Author

I addressed each comment that's been left here so far.

I moved the example implementation from src/regex to examples/regex_engine. This did mean that the final section of the tutorial made no sense anymore as far as I can tell. Instead I replaced that with a section on how to run the example implementation.

tfhe/docs/regex/tutorial.md Outdated Show resolved Hide resolved
tfhe/examples/regex_engine/execution.rs Outdated Show resolved Hide resolved
tfhe/examples/regex_engine/engine.rs Outdated Show resolved Hide resolved
tfhe/examples/regex_engine/parser.rs Outdated Show resolved Hide resolved
tfhe/test_data/client_key Outdated Show resolved Hide resolved
@RKlompUU RKlompUU force-pushed the main branch 3 times, most recently from 0f4c18c to 5e58c34 Compare May 13, 2023 08:28
@aquint-zama
Copy link
Contributor

closes zama-ai/bounty-program#38

RKlompUU and others added 2 commits May 24, 2023 17:28
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.

5 participants