-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Set optimize_file_loading default to False #3513
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Changed default value of optimize_file_loading to False.
Fixed OOM error in backtest by changing optimize_file_loading default value.
|
Which BacktestRunConfig do you use ? |
|
|
|
I mean BacktestDataConfig There's no reason to load everything unless that's how you specifify it. |
That's why I think it's a bug. I think it's preallocating the memory? Here is how I use BacktestDataConfig: |
|
Only directories for the the data you are interested in are loaded, and it's lazy loading from datafusion. If there's a bug the bug should be fixed instead of disabling the feature. Someone else introduced this loading of directories and it seemed to work from his tests. |
|
Hi @wxianxin Thanks for the report and feedback. I agree with @faysou that the underlying behavior needs further investigation. The OOM you experienced suggests the loading isn't as lazy as we might expect, something in the pipeline could be materializing the data eagerly when directories are registered. That said, I think that changing the |
Full respect as I have limited exposure to Nautilus Trader. The new feature is great. It's just the default value caused a bug on a previous normal case, and as long as you are doing some large data backtesting, it will hit OOM and hard to debug. Changing the default value would be the safe option IMO. And the feature is still kept. In current setting, although it appears to be an changeable argument, it's essentially hard-coded settings and the only way to turn it off is to modify the source code. Feel free to close this PR if it's safe to ignore this bug. |
Thanks @xxxsteve. That was also another potential follow-up, to expose this through a config option. |
|
I'll do a pr to expose the parameter as config |
Changed default value of optimize_file_loading to False.
Pull Request
NautilusTrader prioritizes correctness and reliability, please follow existing patterns for validation and testing.
CONTRIBUTING.mdand followed the established practicesSummary
In backtesting, if the backtest data is large enough, setting optimize_file_loading to True will use up all the system memory (and crash the backtest for me). The NT will try to load all the market data into memory before start streaming.
In my case I am doing backtest using Tardis tick and orderbook data for a full month.
Currently there is no way to turn
optimize_file_loadingoff via any config/arg.Type of change
Release notes
RELEASES.mdthat follows the existing conventions (when applicable)Testing
Ensure new or changed logic is covered by tests.