-
Notifications
You must be signed in to change notification settings - Fork 8
Add Insum Lowerer #213
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
Open
TheRealMichaelWang
wants to merge
62
commits into
main
Choose a base branch
from
add-insum
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Add Insum Lowerer #213
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
* Still needs to be tested
* Currently all are failing
…in.Plan * Fixed errors handling parameters in EinsumLowerer
…lues instead of relying on Produce
* Modified pytests accordingly
* Properly handled einsums in return value
* Return values will now be encoded by ein.Produce nodes in the Einsum's bodies tuple
* Reverted to old printing that supports ein.Produce * Fixed misc issues related to reverting changes
* Sparse tensor implementation is COO * Stores an array of coordinates of non-zero elements and an array of non-zero elements
…ith einsum parser naming
…EinsumLowerer and parser
* Get Attribute is a general purpose node that can be used to retreive the coordinate and element arrays from a sparse tensor
* We create a count tensor T, where each element in T is the number of non-zero elements in each corresponding reduced fiber of the sparse tensor * We initialize the initial reduction values based on whether each reduced fiber is all non-zero or not
…plan * Renamed lower_to_pointwise_op to compile_expr and lower_to_pointwise to compile_operand for clarity
… unused rename_einsum method. Updated logic for processing Aggregate cases to improve clarity and efficiency.
…gate queries, improving flexibility in handling various query formats.
…avoid code repitition * Fixed pytests
… inlining its logic directly into the compile_plan method. This change simplifies the code and enhances clarity in handling Aggregate cases.
…e case structure, improving code clarity and reducing redundancy. This change enhances the handling of various query formats, including Reformat and Reorder cases.
…solidating logic into a single case structure, improving code clarity and reducing redundancy. This update also removes outdated code, enhancing maintainability.
This reverts commit 0ebaa91.
* Imported logic AST nodes will be refered to with lgc. prefix prependded for clarity in einsumlowerer
* Will add back in another optimization pass, sometime in the future
… and doesn't rely on building a vector of statements/bodies
Member
willow-ahrens
left a comment
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.
I'm adding my review to remove the request, we've shifted our focus temporarily to the right-hand-side insum PR. Feel free to re-request once the other PR has been merged into main and integrated into this one and this one is ready.
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.
Added a Einsum to Indirect-Einsum (Insum) Lowerer. Additionally the following were also added:
GetAttributeEinsumExpr AST node. It gets an attribute/property of a tensor (i.e. the element or coordinate array of a COO tensor)The Insum Lowerer itself essentially does the following in order to convert an Einsum with a single sparse tensor to an Insum
Twhose elements store the number of non-zero elements in each corresponding fiber of the sparse tensor that is reducedT. We then initialize the initial reduction values of our output tensor based on whether the corresponding reduced fiber is all non-zero or not.f, let the default initial value forfbe calledi, and let the array of elements being reduced calleds. The core assumption we make is that ifshas any zero elements,reduce(f, s) = reduce(f(i, 0), reduce(f, nonzero(s))). Also note that0is not necessarily the only value used inf(i, 0), but we still make a similar assumption.