Improve backtesting data memory reuse#1108
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR updates backtesting data access to reuse cached assets and index structures, refactors ChangesBacktesting performance optimizations and order indexing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant Data
participant DataFrameIndex
Caller->>Data: repair_times_and_fill()
Data->>Data: check dataset size vs _ITER_INDEX_DICT_MAX_ROWS
alt small dataset
Data->>DataFrameIndex: build iter_index_dict
else large dataset
Data->>Data: set iter_index_dict = {}
end
Caller->>Data: access iter_index
Data->>DataFrameIndex: reconstruct pd.Series from df.index
Data-->>Caller: iter_index view
Caller->>Data: to_datalines()
Data->>DataFrameIndex: reuse df.index.array
Data-->>Caller: datetime dataline
sequenceDiagram
participant sync_broker
participant _index_orders_by_identifier
participant BrokerOrders
participant LumibotOrders
sync_broker->>_index_orders_by_identifier: build identifier map from LumibotOrders
_index_orders_by_identifier-->>sync_broker: dict of identifier -> Order or list[Order]
loop for each broker order
sync_broker->>sync_broker: lookup identifier in map
alt list of orders
sync_broker->>sync_broker: reconcile duplicates
else single order
sync_broker->>sync_broker: reconcile single match
end
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Pylint (4.0.6)lumibot/entities/data.py************* Module pylintrc ... [truncated 70083 characters] ... "module": "lumibot.entities.data", lumibot/strategies/_strategy.py************* Module pylintrc ... [truncated 187522 characters] ... ng", Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lumibot/entities/data.py (1)
1367-1384: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftNative fast paths should keep the same data-availability guard
The
native_multi_minuteandnative_1branches still call_get_bars_frame_window()directly, so they bypass thestrict_end_check/stale-bar handling and "outside available data" warnings that the resample path gets from_validate_bars_request(). If this is intentional, it needs its own guard; otherwise call_validate_bars_request()before taking the fast path. [end]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lumibot/entities/data.py` around lines 1367 - 1384, The native fast path in the data retrieval logic is bypassing the same availability checks used by the resample path. Update the `native_multi_minute`/`native_1` handling in `Data` so it runs `_validate_bars_request()` before calling `_get_bars_frame_window()`, or add an equivalent guard there if the fast path must remain separate. Make sure the existing `strict_end_check`, stale-bar behavior, and “outside available data” warning semantics stay consistent across these branches.
🧹 Nitpick comments (1)
lumibot/entities/data.py (1)
1384-1443: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated NaN/column-presence post-processing block across both native fast paths.
The ~40-line block handling
_bars_has_volume/_bars_has_dividend/_bars_has_stock_splitscaching, NaN fill, anddropnais now duplicated verbatim between the "native_multi_minute" and "native_1" fast paths. Since this refactor already touched both blocks to swap in_get_bars_frame_window(), extracting this shared post-processing into a small helper (e.g._finalize_bars_slice(df)) would reduce future drift risk between the two paths.Also applies to: 1488-1531
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lumibot/entities/data.py` around lines 1384 - 1443, The NaN/column-presence post-processing logic is duplicated in both native fast paths, which risks drift between the "native_multi_minute" and "native_1" branches. Extract the shared block that uses _bars_has_volume, _bars_has_dividend, _bars_has_stock_splits, _bars_required_cols, and the NaN-fill/dropna logic into a small helper such as _finalize_bars_slice(df), then call that helper from both code paths. Keep the cache key behavior and returned DataFrame semantics unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lumibot/entities/data.py`:
- Around line 1300-1324: The `_get_bars_frame_window` helper currently depends
on pandas’ private `_slice()` method, which can break on future pandas changes.
Update this logic to use the public indexing path in `_get_bars_frame_window` on
the cached source frame returned by `_get_bars_source_frame`, using a
`.iloc[start_row:end_row]` fallback for the window extraction. Keep the existing
`start_row`, `end_row`, and `normalized_timeshift` behavior intact so
`get_bars()` continues to return the same results.
---
Outside diff comments:
In `@lumibot/entities/data.py`:
- Around line 1367-1384: The native fast path in the data retrieval logic is
bypassing the same availability checks used by the resample path. Update the
`native_multi_minute`/`native_1` handling in `Data` so it runs
`_validate_bars_request()` before calling `_get_bars_frame_window()`, or add an
equivalent guard there if the fast path must remain separate. Make sure the
existing `strict_end_check`, stale-bar behavior, and “outside available data”
warning semantics stay consistent across these branches.
---
Nitpick comments:
In `@lumibot/entities/data.py`:
- Around line 1384-1443: The NaN/column-presence post-processing logic is
duplicated in both native fast paths, which risks drift between the
"native_multi_minute" and "native_1" branches. Extract the shared block that
uses _bars_has_volume, _bars_has_dividend, _bars_has_stock_splits,
_bars_required_cols, and the NaN-fill/dropna logic into a small helper such as
_finalize_bars_slice(df), then call that helper from both code paths. Keep the
cache key behavior and returned DataFrame semantics unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e4d43f3f-0cb8-45e0-a84b-96a2288ef570
📒 Files selected for processing (8)
CHANGELOG.mdlumibot/data_sources/pandas_data.pylumibot/entities/data.pylumibot/strategies/strategy_executor.pytests/test_data_entity.pytests/test_memory_efficiency_entities_backtesting.pytests/test_pandas_data_find_asset_timestep_match.pytests/test_strategy_executor_order_index.py
1 participant
Summary
DatetimeIndexbacking array forDatadatetime datalines instead of allocating a duplicate object ndarray during repair.Benchmark
Local harness: 120k 1-minute OHLCV rows, 20k
get_last_price()calls, 4kget_bars()calls.Data.repair_times_and_fill()median: 0.378902s before -> 0.006489s afterget_last_price()loop: 0.077269s before -> 0.078164s afterget_bars()loop: 0.055513s before -> 0.054401s afterTests
.venv/bin/python -m pytest tests/test_memory_efficiency_entities_backtesting.py tests/test_data_entity.py tests/test_data_iter_count_microsecond_index.py tests/test_pandas_data_find_asset_timestep_match.py.venv/bin/python -m pytest tests/test_get_historical_prices.py tests/test_data_timeshift_none.py tests/test_data_get_bars_day_includes_latest_completed_bar.py tests/test_backtesting_pandas_daily_routing.py.venv/bin/python -m pytest tests/test_data_source.py tests/test_backtesting_broker.pySummary by CodeRabbit
Databar slicing/resampling to avoid retaining large timestamp/index structures when not needed.repair_times_and_fill()cases, order indexing (including duplicates), and resampling with extra columns.