compiler: Faster compilation time#2949
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2949 +/- ##
==========================================
+ Coverage 83.47% 83.60% +0.13%
==========================================
Files 249 250 +1
Lines 52276 52916 +640
Branches 4503 4568 +65
==========================================
+ Hits 43638 44243 +605
- Misses 7880 7894 +14
- Partials 758 779 +21
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
mloubout
left a comment
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.
|
|
||
| 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
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
| 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
There was a problem hiding this comment.
yeah codex likes that too
FabioLuporini
force-pushed
the
faster-python-rebased-final
branch
from
c38c64a to
21f8660
Compare
FabioLuporini
force-pushed
the
faster-python-rebased-final
branch
from
9499f27 to
683a23c
Compare
FabioLuporini
force-pushed
the
faster-python-rebased-final
branch
from
683a23c to
8fc9ca5
Compare
| else: | ||
| return [ClusterGroup(processed, prefix)] | ||
|
|
||
| class Key(tuple): |
There was a problem hiding this comment.
I agree, will change this
| return exprs | ||
| return replaced | ||
| else: | ||
| return replaced[0] |
2 participants
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