[perf]: replace eval() with direct comparison in check_param#499
Open
vTusharr wants to merge 1 commit intodswah:mainfrom
Open
[perf]: replace eval() with direct comparison in check_param#499vTusharr wants to merge 1 commit intodswah:mainfrom
vTusharr wants to merge 1 commit intodswah:mainfrom
Conversation
6e787e3 to
cc75ebc
Compare
- Replace eval() with direct comparison in check_param to avoid evaluation overhead - Defer exception message building to avoid costly repr() and string formatting overhead Signed-off-by: Tushar Verma <tusharVermaiota@proton.me>
Owner
|
@vTusharr Cool thanks for the PR. |
Author
|
@dswah Yeah , was kinda busy with exams & all , Old code was calling eval() and building error message strings on all calls, even when validation passed , during a gridsearch that's a lot of calls , so i made it so that only when it fails the string is constructed and replaced eval("np." + repr(param_dt) + constraint) with a dict of pre-built lambdas |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace eval('np.' + repr(param_dt) + constraint) with a lookup dict of direct numpy comparison operators in check_param(). This eliminates many calls to eval() and repr() per run ; Falls back to eval() for unknown constraint strings to maintain backwards compatibility.
Summary
check_param()previously built ->msg = param_name + f" = {repr(param)}"at the start of the function. This forced to compute the string and run
repr(param)on every call, even when no error occurred.Change
The message construction is moved into a nested helper:
Error paths now call it only when needed:
###why?
This uses lazy evaluation while keeping the code DRY . Instead of computing the message on every call or duplicating the string-building code across multiple error sites, it is built only when an exception is raised.
###now
Avoids unnecessary
repr(param)and string formatting in the common case where parameters are valid, removing overhead from the hot path and producing a significant speedup.py-spy flame graph
before
after
Profiling methodology
predict, intervals, sampling, partial dependence, constraints, etc.)
tests 162 passed, 1 skipped, 62 warnings in 30.35s