-
Notifications
You must be signed in to change notification settings - Fork 15
Add implementation-specific descriptors #343
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
Conversation
I decided to normalize everything to lowercase and raise if this causes duplicates. Since descriptor options are keyword arguments, they can potentially overlap with other argument names, and a workaround is to use a capitalized version of the keyword for the descriptor.
I decided to create the descriptor, then ignore it. This at least ensures that the descriptor options were valid, which will help us transition to when descriptors can be used. @jim22k, auto-compute of expressions leads to an interesting edge case when it comes to custom descriptors. Suppose an expression is used causing it to auto-compute (and the result is stored as e.g. I'm going to merge soon--gonna do one final scan and check coverage. |
This is in! This is a significant addition, and it feels great to close an issue that is 2.5 years old 😃 |
Closes #20.
@jim22k, I have a few questions:
axb_method=
orAxB_method=
?graphblas/core/ss/descriptor.py:get_descriptor
, but didn't add it to the docs website. I plan to make a new issue for documenting backend-specific descriptors. Sound good?I think this PR covers pretty much everywhere custom descriptors may be used.
Also, I got rid of caching of custom descriptors. We previously did this b/c descriptors weren't cleaned up properly, not for speed (probably). Now they are cleaned up properly.