pymbolic
pymbolic copied to clipboard
Add EqualityMapper
Added an EqualityMapper similar to the one in inducer/pytato#166.
This seems a bit slower than the current version (benchmarked against inducer/pytential#121): it went from 5.3s-ish to 6.6s-ish.
- [x] Rebase once #72 is in to remove extra commits.
- [x] Need to figure out why it's so slow (not caching properly?).
- [x] Add test that this is actually caching.
- [x] Add benchmark of some sort that it actually makes things better in some pathological cases?
- [ ] Update and pass tests in downstreams. Current work at
- [ ] https://github.com/inducer/loopy/pull/607
- [ ] https://github.com/inducer/pytential/pull/148
- [ ] https://github.com/inducer/pytato/pull/317
- [x] https://github.com/inducer/grudge/pull/255
- [ ] Point CI back to the proper downstream branches.
- [ ] Remove
fail-fastfrom downstream CI
Fixes #73.
@kaushikcfd Is this worth pursuing given that compile times are suffering because of expression traversal complexity in pymbolic?
@kaushikcfd Is this worth pursuing given that compile times are suffering because of expression traversal complexity in pymbolic?
For what it's worth, the loopy branch is up to date and passing tests locally, so it shouldn't be too hard to clean this up.
Is this worth pursuing given that compile times are suffering because of expression traversal complexity in pymbolic?
I think this is useful. IIUC, this PR quite significantly brings down the complexity of the expression comparison. If the overheads of this approach (creating a new mapper for every comparison) are under-control, I would love to get rid of hacks such as: https://github.com/kaushikcfd/pymbolic/blob/a697d47cd53e9902abc49f98d67921b466c992c2/pymbolic/mapper/init.py#L213.
However, I wonder how the loopy CI failures could be fixed? By creating a derived version of every pymbolic expression type in loopy so that they can use loopy-specific EqualityMapper?
However, I wonder how the loopy CI failures could be fixed? By creating a derived version of every pymbolic expression type in loopy so that they can use loopy-specific EqualityMapper?
It should work with the modified branches. I imagine it needs some work in ci-support.sh to clone the correct repo and branch (instead of hardcoding to inducer/loopy@main).
Oops had missed the branch in the description. I think those will work fine. Thanks!
I would love to get rid of hacks such as:
Whoa. I had missed that during review. That is pretty gross.
Add a test like https://github.com/kaushikcfd/pytato/blob/1d315637ee237631b30a24228375c33257ea7dbe/test/test_pytato.py#L307-L330
I need to do some measuring.