⚡ Optimize arg_alias and fix string alias bug#80
Conversation
- Optimized `arg_alias` decorator by pre-processing the alias dictionary once during decoration, removing iteration and type checks from the function call path. - Fixed a bug where single string aliases were ignored (runtime) and caused `re.error` (docstring generation). - Added a regression test case in `tests/test_miscs.py`. Benchmarks show ~40% performance improvement (1.2s vs 2.0s for 1M calls). Co-authored-by: refraction-ray <35157286+refraction-ray@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
tests/test_miscs.py
Outdated
| """ | ||
| return theta | ||
|
|
||
| assert g(alpha=1.0) == 1.0 |
There was a problem hiding this comment.
Do not perform equality checks with floating point values.
There was a problem hiding this comment.
Good catch! I've updated the test to use np.testing.assert_allclose for the floating point comparison.
Updated the new test case in `tests/test_miscs.py` to use `np.testing.assert_allclose` instead of equality check for floating point values, as requested in code review. Co-authored-by: refraction-ray <35157286+refraction-ray@users.noreply.github.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
1 participant
This PR optimizes the
arg_aliasdecorator intensorcircuit/utils.pyand fixes a bug handling single string aliases.Optimization:
The original implementation iterated over
alias_dict.items()inside the wrapper function, performingisinstancechecks on every call.The new implementation pre-processes
alias_dictinto a flat list of(key, alias)tuples at decoration time. The wrapper then iterates over this flat list, removing the overhead from the hot path.Benchmarks indicate a ~40% speedup (from ~2.0s to ~1.2s for 1,000,000 calls).
Bug Fix:
The original implementation had logic
if isinstance(vs, str): vs = [], which effectively discarded single string aliases.The new implementation correctly normalizes single string aliases into a list
[vs].This also fixes a crash in docstring generation where iterating over a single string (alias name) caused
re.errorwhen characters like digits were encountered inre.sub.Verification:
tests/test_miscs.pycovering the single string alias scenario.PR created automatically by Jules for task 18006180529273122311 started by @refraction-ray