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

Quality of Life Pass/Pipeline Features #1107

Closed
6 of 8 tasks
AlexanderViand-Intel opened this issue Nov 22, 2024 · 5 comments · Fixed by #1196
Closed
6 of 8 tasks

Quality of Life Pass/Pipeline Features #1107

AlexanderViand-Intel opened this issue Nov 22, 2024 · 5 comments · Fixed by #1196
Labels
good first issue Good for newcomers

Comments

@AlexanderViand-Intel
Copy link
Collaborator

AlexanderViand-Intel commented Nov 22, 2024

Small fixes/add-ons that I noticed while drawing up a pass/pipeline overview figure:

  • --secretize's entry-function parameter should be optional and, if omitted, the pass should apply to all functions in the input (module?). This should then also be reflected in any pipelines that use --secretize, making their entry-function optional, too.
  • In order to better support mixed ctxt/ptxt examples, and for better integration with frontends, create --annotated-mlir-to-.... that are the same as the current --mlir-to-... pipelines but without the --secretize pass.
  • The ciphertext-degree parameter in the various passes & pipelines that use it should be optional, with the compiler using the size of (the right-most-non-one-dimension of) the first secret tensor in the function signature if the parameter is omitted (and erroring out if no such secret tensor function parameter exists)
    See: Encode (R)LWE Parameters into the IR #1197
  • --optimize-relinearization should be added to the RLWE pipeline builder? (EDIT: Not until --optimize-relinearization crashing #1148, and ideally also Generalize OptimizeRelinearization to support schemes besides BGV #1032 are fixed)
  • --operation-balancer shuld be added to the mlir-to-secret-arithmetic pipeline
  • Investigate if the --bgv-to-openfhe and --ckks-to-openfhe passes can be merged into a single --rlwe-to-openfhe pass combining the patterns from both passes.
    Also: Unify generic "convert binary op" patterns #1150
  • (Optional/TBD): Split the library-specific parts of the pipeline into separate pipelines. While that might require doing something like --mlir-to-ckks --rlwe-to-openfhe, it might be nicer to keep backend-specific stuff separate/modular.
  • Better error messages ( Improve Error/Warning Messages in HEIR through more meaningful Op "location" #1143)
@AlexanderViand-Intel AlexanderViand-Intel added the good first issue Good for newcomers label Nov 22, 2024
@AlexanderViand-Intel
Copy link
Collaborator Author

Based on discussion from today's office hours:

For the ciphertext-degree parameter, we should decouple that into the actual ctxt degree (which can be optional and default to something reasoanble such as 16k) and something like --assume-native-tensor-size that does what was mentioned above.

In general, these things should not be just stored in a single pass's paramaters but encoded into the IR (at the module level)

@ZenithalHourlyRate
Copy link
Collaborator

Also I'm wondering why --operation-balancer did not get into the mlir-to-secret-arithmetic pipeline, though its absense makes me happy when constructing deep circuit for debugging.

@AlexanderViand-Intel
Copy link
Collaborator Author

Also I'm wondering why --operation-balancer did not get into the mlir-to-secret-arithmetic pipeline, though its absense makes me happy when constructing deep circuit for debugging.

Added it to the list! Do you happen to know which tests you expect to break?

@ZenithalHourlyRate
Copy link
Collaborator

Do you happen to know which tests you expect to break?

No, haven't tried it.

@AlexanderViand-Intel
Copy link
Collaborator Author

With #1196 addressing the last "clean up" part of this and the ciphertext degree thing + error messages being quite significant work packages in their own right with their own issues to track them, I've set this to close when #1196 gets merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants