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

Polynomial approximation patterns #1449

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

j2kun
Copy link
Collaborator

@j2kun j2kun commented Feb 22, 2025

Part of #1217

This PR adds polynomial approximation patterns for all unary math dialect ops that operate on floats, e.g., math.exp. These should all be elementwise mappable, so extent to tensors and similar for use in the secret+tensor_ext_layer.

The pass supports extracting the desired degree/interval of approximation from the op attributes, and otherwise uses a default [-1, 1] degree 5 approximation.

What's missing:

  • Smarter error handling (e.g., someone asks for an approximation of an op outside its domain)
  • Binary/ternary math ops, when all but one argument is statically known (filed a TODO)
  • Patterns that match on higher level math ops, e.g., stablehlo.relu lowers to something like linalg.map { arith.maximumf }

@j2kun j2kun marked this pull request as draft February 22, 2025 20:13
@j2kun j2kun force-pushed the poly-approx-patterns branch 3 times, most recently from b44e84c to afe1a58 Compare February 25, 2025 17:57
@j2kun j2kun marked this pull request as ready for review February 27, 2025 18:42
Copy link
Collaborator

@AlexanderViand-Intel AlexanderViand-Intel left a comment

Choose a reason for hiding this comment

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

Amazing! Just a nit and one real request: could you also add this pass to the --mlir-to-secret-arithmetic pipeline, and add at least one math.<something> op in an e2e test? Thanks!

@j2kun
Copy link
Collaborator Author

j2kun commented Feb 27, 2025

could you also add this pass to the --mlir-to-secret-arithmetic pipeline, and add at least one math.<something> op in an e2e test? Thanks!

I actually cannot, because we don't have a lowering for polynomial.eval yet.

Copy link
Collaborator

@AlexanderViand-Intel AlexanderViand-Intel left a comment

Choose a reason for hiding this comment

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

could you also add this pass to the --mlir-to-secret-arithmetic pipeline, and add at least one math.<something> op in an e2e test? Thanks!

I actually cannot, because we don't have a lowering for polynomial.eval yet.

:( That's a good reason, though xD

@j2kun j2kun force-pushed the poly-approx-patterns branch from 7d1d584 to bb17c79 Compare February 27, 2025 22:02
@j2kun j2kun added the pull_ready Indicates whether a PR is ready to pull. The copybara worker will import for internal testing label Feb 27, 2025
@copybara-service copybara-service bot merged commit 6c1bd2e into google:main Feb 28, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull_ready Indicates whether a PR is ready to pull. The copybara worker will import for internal testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants