-
Notifications
You must be signed in to change notification settings - Fork 48
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
[tk] Switch symbolic/shape placeholders to sympy #375
Conversation
@Groverkss This is passing the tests post my refactor to:
I dropped the SliceAnalysis thing and inlined it into some more direct cast utilities. I'm going to keep iterating on this a little bit because now I can add the stuff at the top level to directly make symbols usable in the IR. |
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 like this a lot. I think I have some doubts about carrying the range analysis with the value and about slicing, but I think we can fix them later.
Let's land this and we can iterate on this further.
Thanks. I don't like where that was going either. I needed it when I was in the ditch trying to make slice analysis robust... But then I just deleted it and went with something more direct. This revision still needs this for the moment for accessing dynamic grid values, but I don't like that and will scalpel it out. |
By using sympy instead of the stand-in symbolic support, we get full support for index (partial) evaluation, dynamic dimensions and validation checks.