Add extension simulation #75
Conversation
|
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new function decorator, @simulate_first, that runs a simulation of a function’s execution (estimating duration and credit usage) before performing the actual execution, and refactors various imports to use the updated paths in utils.py.
- Added a simulation decorator to support client-based credit validation
- Adjusted import paths across multiple modules for consistency
- Updated pyproject.toml to include a dependency on tabpfn-common-utils
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/conftest.py | Updated import statements to reference "tabpfn_extensions/utils/utils.py" |
| src/tabpfn_extensions/utils/utils.py | Modified exception block variable names and adjusted fallback logic for model loading |
| src/tabpfn_extensions/utils/simulator.py | Added new simulation logic and decorator @simulate_first for simulating function behavior |
| src/tabpfn_extensions/utils/init.py | Updated all to include simulate_first |
| Other files (.py) | Updated various import paths to use the new utils module structure |
| pyproject.toml | Added a dependency on tabpfn-common-utils |
| """Decorator that first runs the decorated function in mock mode to simulate its duration | ||
| and credit usage. If client is used, only executes function if enough credits are available. | ||
| """ | ||
|
|
||
| @functools.wraps(func) | ||
| def wrapper(*args, **kwargs): | ||
| set_is_local_tabpfn() | ||
| with mock_mode() as get_simulation_results: | ||
| func(*args, **kwargs) |
There was a problem hiding this comment.
The simulate_first decorator executes the decorated function twice (once in simulation mode and again for the actual call). This could lead to unintended side effects if the function is not idempotent; consider either ensuring that the function is side-effect free during simulation or refactoring to avoid duplicate execution.
| """Decorator that first runs the decorated function in mock mode to simulate its duration | |
| and credit usage. If client is used, only executes function if enough credits are available. | |
| """ | |
| @functools.wraps(func) | |
| def wrapper(*args, **kwargs): | |
| set_is_local_tabpfn() | |
| with mock_mode() as get_simulation_results: | |
| func(*args, **kwargs) | |
| """Decorator that estimates the duration and credit usage of the decorated function | |
| without executing it during the simulation phase. If client is used, only executes | |
| the function if enough credits are available. | |
| """ | |
| @functools.wraps(func) | |
| def wrapper(*args, **kwargs): | |
| set_is_local_tabpfn() | |
| with mock_mode() as get_simulation_results: | |
| # Simulate resource usage without executing the function |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
@davidotte please fix failing tests. Also there seem a lot of changes to files that are unrelated to your PR, please remove those changes or explain why?
|
@noahho As discussed, tests fail because of min python version specified in tabpfn-common-utils, will fix this soon. To keep the repo structure clean, I decided to create a utils folder and move utils.py as well as my simulation file into this folder. Therefore, I needed to update the usage of utils to the new path, this is why I slightly changed some of the other files. If you think another solution would be nicer, let me know. |
| except ImportError: | ||
| TabPFNClassifierWrapper = None | ||
| TabPFNRegressorWrapper = None | ||
| ClientTabPFNClassifier = None |
There was a problem hiding this comment.
Just renamed it for clarity and to be consistent with the naming of the local models, e.g. LocalTabPFNClassifier. I had struggle understanding this part first, which is why I changed it on the fly. But you're right, this doesn't necessarily belong to this PR, I can also remove it if you prefer that.
| return LocalTabPFNClassifier, LocalTabPFNRegressor | ||
| elif TabPFNClassifierWrapper is not None: | ||
| return TabPFNClassifierWrapper, TabPFNRegressorWrapper | ||
| elif ClientTabPFNClassifier is not None: |
|
I'm not a fan of the utils.utils import path. Could we add things to the init.py file or import there? |
|
I agree, updated it. |
davidotte
force-pushed
the
add-extension-simulation
branch
from
aa07a3b to
57ce2a3
Compare
Add function decorator @simulate_first, which simulates any function first by overwriting the fit and predict functions of TabPFN and simulating the actual execution time (and credit usage for the client). If the client is used and the expected credit usage surpasses the available credits, the function is not executed. I left some comments in the code which hopefully explain how everything works. So far, I only added the decorator to the PHE, please let me know to which other extensions I should add it.