compiler: Faster compilation time - #2949
Conversation
Sorry, something went wrong.
Codecov Report❌ Patch coverage is 92.67161% with 79 lines in your changes missing coverage. Please review. @@ Coverage Diff @@
## main #2949 +/- ##
==========================================
+ Coverage 82.86% 83.60% +0.73%
==========================================
Files 249 250 +1
Lines 52268 52908 +640
Branches 4499 4564 +65
==========================================
+ Hits 43314 44235 +921
+ Misses 8184 7895 -289
- Partials 770 778 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness.
|
Sorry, something went wrong.
mloubout
left a comment
There was a problem hiding this comment.
Not sure I like some of those intricate caching methods that could use better implementation rather than a new caching method every place it needs something a bit different.
Sorry, something went wrong.
|
|
||
| def pow_to_mul(expr): | ||
| if q_leaf(expr) or isinstance(expr, Basic): | ||
| if type(expr) in (tuple, list): |
There was a problem hiding this comment.
This should check if it's Iterable to handle generators and such
Sorry, something went wrong.
There was a problem hiding this comment.
Checking if Iterable introduces other issues, since str/dict/... are also Iterable
I agree that using type(expr) is ugly though, I'll switch it to isinstance
Sorry, something went wrong.
| body = iet.body._rebuild(body=iet.body.body + (DummyExpr(x, x),)) | ||
| return iet._rebuild(body=body), {} | ||
|
|
||
| monkeypatch.setattr(iet_engine, '_update_args', |
There was a problem hiding this comment.
Claude is driving me crazy with these monkeypatch 😅 it's always what it tries first for no reason. Like it had 80 of those or something in horizon
Sorry, something went wrong.
There was a problem hiding this comment.
yeah codex likes that too
Sorry, something went wrong.
mloubout
left a comment
There was a problem hiding this comment.
Some more comments 😬
Sorry, something went wrong.
| else: | ||
| return [ClusterGroup(processed, prefix)] | ||
|
|
||
| class Key(tuple): |
There was a problem hiding this comment.
doesn't need to be local
Sorry, something went wrong.
There was a problem hiding this comment.
I agree, will change this
Sorry, something went wrong.
| return exprs | ||
| return replaced | ||
| else: | ||
| return replaced[0] |
There was a problem hiding this comment.
if replaced[0] == exprs?
Sorry, something went wrong.
the key ingredients are:
Bonus: a lot of, IMHO, good refactoring. And several new tests.
Compilation time improvements vary between 1.2x and 5x depending on the complexity of the Operator