-
Notifications
You must be signed in to change notification settings - Fork 109
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
Allow out-of-crate preprocessed column definitions #939
base: dev
Are you sure you want to change the base?
Conversation
8c03507
to
cec714b
Compare
I got early feedbacks. I want to give it another try
This would eliminate the dynamic dispatch business, and will be a straightforward generalization of the current codebase. |
Didn't quite work out. I am adding tons of type arguments everywhere. At the moment this PR is still my best try. |
are we going to have a function for every backend? Code quote: fn gen_preprocessed_column_cpu(
&self,
) -> CircleEvaluation<CpuBackend, BaseField, BitReversedOrder>;
fn gen_preprocessed_column_simd(
&self,
) -> CircleEvaluation<SimdBackend, BaseField, BitReversedOrder>; |
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.
Reviewed 3 of 14 files at r1, all commit messages.
Reviewable status: 3 of 14 files reviewed, 1 unresolved discussion (waiting on @yoichi-nexus)
Yes, that will happen in this approach. Passing Alternatives:
|
This PR removes an enum
PreprocessedColumn
and introduces a traitPreprocessedColumnOps
. The enum values are turned into trait implementations.Motivation
Before this PR, when a
stwo
user wanted a new column in the preprocessed column, they had to changestwo
codebase and add a value inPreprocessedColumn
enum.After this PR, they can implement
PreprocessedColumnOps
in their own crate, without touchingstwo
codebase.