Feat/pipeline optimizations - #30
Conversation
…ate selection logic
…message formatting
…figs are properly evaluated and gated
There was a problem hiding this comment.
Code Review
This pull request introduces a cheap in-sample pre-screen to skip expensive statistical evaluations for poor candidates, and an Optuna-based numeric parameter optimization feature to tune strategy parameters on in-sample data. It also updates LLM prompts to display unified diffs of failed attempts and adds database schema updates and tests. The review feedback focuses on improving the performance of the Optuna search by pre-computing walk-forward windows and asset returns, securing YAML serialization with safe_dump, robustly handling NaN values in metric comparisons, and simplifying file cleanup with unlink(missing_ok=True).
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def _in_sample_objective( | ||
| generate_signals_fn: Any, | ||
| flat_config: dict[str, Any], | ||
| prices: pd.DataFrame, | ||
| ) -> tuple[float, float, pd.Series]: |
There was a problem hiding this comment.
To significantly improve the efficiency of the Optuna parameter search, we can avoid re-computing the walk-forward windows and asset returns on every single trial. Since prices and settings do not change during the optimization process, these values can be pre-computed once and passed into _in_sample_objective as optional arguments.
def _in_sample_objective(
generate_signals_fn: Any,
flat_config: dict[str, Any],
prices: pd.DataFrame,
_asset_returns: pd.DataFrame | None = None,
_wf_windows: list[Any] | None = None,
) -> tuple[float, float, pd.Series]:| in_sample_idx, _ = partition_holdout_data(prices.index, holdout_years=settings.default_holdout_years) | ||
| wf_windows = generate_walk_forward_windows(in_sample_idx, train_years=5, test_years=1) | ||
| if not wf_windows: | ||
| return 0.0, 0.0, pd.Series(dtype=float) | ||
|
|
||
| wf_weights = _generate_wf_weights(prices, generate_signals_fn, flat_config, tickers, wf_windows) | ||
| _asset_returns = prices.pct_change().fillna(0.0) |
There was a problem hiding this comment.
Use the optional pre-computed _wf_windows and _asset_returns if provided, to bypass the expensive window generation and pct_change() calculations during iterative optimization trials.
if _wf_windows is None:
in_sample_idx, _ = partition_holdout_data(prices.index, holdout_years=settings.default_holdout_years)
wf_windows = generate_walk_forward_windows(in_sample_idx, train_years=5, test_years=1)
else:
wf_windows = _wf_windows
if not wf_windows:
return 0.0, 0.0, pd.Series(dtype=float)
wf_weights = _generate_wf_weights(prices, generate_signals_fn, flat_config, tickers, wf_windows)
if _asset_returns is None:
_asset_returns = prices.pct_change().fillna(0.0)| original_sharpe, _, _ = _in_sample_objective(generate_signals_fn, flat_config, prices) | ||
| if pd.isna(original_sharpe): | ||
| original_sharpe = 0.0 |
There was a problem hiding this comment.
Pre-compute the walk-forward windows and asset returns once before starting the Optuna study, and pass them to _in_sample_objective to avoid redundant computations in every trial.
| original_sharpe, _, _ = _in_sample_objective(generate_signals_fn, flat_config, prices) | |
| if pd.isna(original_sharpe): | |
| original_sharpe = 0.0 | |
| from autobacktest.config import settings | |
| from autobacktest.evaluator.evaluate import partition_holdout_data, generate_walk_forward_windows | |
| in_sample_idx, _ = partition_holdout_data(prices.index, holdout_years=settings.default_holdout_years) | |
| wf_windows = generate_walk_forward_windows(in_sample_idx, train_years=5, test_years=1) | |
| asset_returns = prices.pct_change().fillna(0.0) | |
| original_sharpe, _, _ = _in_sample_objective( | |
| generate_signals_fn, flat_config, prices, _asset_returns=asset_returns, _wf_windows=wf_windows | |
| ) | |
| if pd.isna(original_sharpe): | |
| original_sharpe = 0.0 |
| sh, _, _ = _in_sample_objective(generate_signals_fn, trial_config, prices) | ||
| if pd.isna(sh): | ||
| return -float("inf") | ||
| return float(sh) |
There was a problem hiding this comment.
Pass the pre-computed asset_returns and wf_windows to _in_sample_objective inside the Optuna objective function to prevent recalculating them on every trial.
| sh, _, _ = _in_sample_objective(generate_signals_fn, trial_config, prices) | |
| if pd.isna(sh): | |
| return -float("inf") | |
| return float(sh) | |
| sh, _, _ = _in_sample_objective( | |
| generate_signals_fn, trial_config, prices, _asset_returns=asset_returns, _wf_windows=wf_windows | |
| ) | |
| if pd.isna(sh): | |
| return -float("inf") | |
| return float(sh) |
| params = optimized_config.get("params", {}) | ||
| if isinstance(params, dict) and params: | ||
| yaml_dict["params"] = params | ||
| return yaml.dump(yaml_dict, default_flow_style=False, sort_keys=False) |
There was a problem hiding this comment.
Use yaml.safe_dump instead of yaml.dump to ensure secure and standard serialization of the configuration dictionary, avoiding potential execution of arbitrary Python objects.
| return yaml.dump(yaml_dict, default_flow_style=False, sort_keys=False) | |
| return yaml.safe_dump(yaml_dict, default_flow_style=False, sort_keys=False) |
| for p in [temp_py, temp_yaml]: | ||
| if p.exists(): | ||
| p.unlink() |
| orig_metric = _get_metric_value(winner["_report"], self.target_metric) | ||
| opt_metric = _get_metric_value(opt_report, self.target_metric) | ||
|
|
||
| if sel.accepted and cnf.accepted and opt_metric >= orig_metric: |
There was a problem hiding this comment.
If either opt_metric or orig_metric is NaN, direct comparison using >= will evaluate to False. To make the comparison robust against NaN values, default them to -inf before comparing.
| if sel.accepted and cnf.accepted and opt_metric >= orig_metric: | |
| orig_val = orig_metric if not pd.isna(orig_metric) else -float("inf") | |
| opt_val = opt_metric if not pd.isna(opt_metric) else -float("inf") | |
| if sel.accepted and cnf.accepted and opt_val >= orig_val: |
…puted walk-forward windows and asset returns
No description provided.