Implement a model to determine when it's better to use a linear merge sum rather than the standard quadratic merge sum #8
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sum rather than the standard quadratic merge sum Use the model to reduce adaptive evalution costs, deciding at compile time whether the linear merge sums + required lower level merge sums is better than just using the top level quadratic merge sum Correct the estimated quadratic merge latency model to account for the quadratic merge assuming neither sums have been built as non-overlapping and non-adjacent Add eval_type to the latency estimates, needed for two_sum which may has different characteristics for `vector_type` outputs TODO: Fix the failing adaptive test cases
Detect if subtrees are merged or not when using the linear merge, if not, then merge them individually before performing the linear merge Always perform a linear merge higher levels of the expression tree are expecting it Implement and use sparse_mult_merge, the algorithm used to multiply two partial sums of strongly non-adjacent values with the result being strongly non-adjacent Apparently slightly bugged, need to diagnose the issue
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8 +/- ##
==========================================
+ Coverage 98.06% 98.14% +0.08%
==========================================
Files 13 14 +1
Lines 1034 1241 +207
==========================================
+ Hits 1014 1218 +204
- Misses 20 23 +3 ☔ View full report in Codecov by Sentry. |
Also add zero-pruning to the (slow) linear merge, gives substantial performance improvements (~150 us to ~15 us)
Probably incurs a performance penalty to expressions using it Also move common helful functors like is_nonzero and zero_prune_store to utils
More cleanup, move copy_nonzero to utils, use zero_prune_store_inc some more Split out the binary recursive merge from the sparse_mult_merge method, implement it as the standard recursive merge + zero pruning Use somewhat better names in sparse_mult_merge_term
mfdeakin
force-pushed
the
adaptive_linear_merge
branch
from
bb7f7c2 to
7d86934
Compare
Add multiplication implementation tests Remove zero_prune_store_dec, rename zero_prune_store_inc to zero_prune_store More uses of zero_prune_store instead of copy-pasta
Now do only necessary work, don't add, negate, or multiply zeros Remove non-zero filtering
mfdeakin
force-pushed
the
adaptive_linear_merge
branch
from
6a36e71 to
562261c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use the model to reduce adaptive evalution costs, deciding at compile time whether the linear merge sums + required lower level merge sums is better than just using the top level quadratic merge sum
Correct the estimated quadratic merge latency model to account for the quadratic merge assuming neither sums have been built as non-overlapping and non-adjacent
Add eval_type to the latency estimates, needed for two_sum which may has different characteristics for
vector_typeoutputs