mhallsmoore / qstrader

QuantStart.com - QSTrader backtesting simulation engine.
https://www.quantstart.com/qstrader/
MIT License
2.84k stars 851 forks source link

AssertionError in test_backtest_e2e.py #349

Closed b4d0n3 closed 3 years ago

b4d0n3 commented 3 years ago

Running the following Dockerfile produces $subject (easy reproducer :) ).

# BUILD:  docker build -t qstrader-focal .
# RUN:    docker run -it -v "$PWD":/qstrader-data qstrader-focal

FROM ubuntu:focal

RUN apt-get update && apt-get install -y git python3-pip
RUN git clone https://github.com/mhallsmoore/qstrader.git
RUN pip3 install -r qstrader/requirements/base.txt
RUN pip3 install -r qstrader/requirements/tests.txt
WORKDIR /qstrader
ENV PYTHONPATH /qstrader
RUN pytest
        pd.testing.assert_frame_equal(history_df, expected_df)                                                                                                                                                                              
>       assert portfolio_dict == expected_dict                                                                                                                                                                                              
E       AssertionError: assert {'EQ:ABC': {'...1644671, ...}} == {'EQ:ABC': {'...6446715, ...}}                                                                                                                                             
E         Omitting 1 identical items, use -vv to show                                                                                                                                                                                       
E         Differing items:                                                                                                                                                                                                                  
E         {'EQ:DEF': {'market_value': 376203.80367208034, 'quantity': 1431.0, 'realised_pnl': 613.3956570402876, 'total_pnl': 18661.22701644671, ...}} != {'EQ:DEF': {'market_value': 376203.80367208034, 'quantity': 1431.0, 'realised_pnl'
: 613.3956570402925, 'total_pnl': 18661.227016446715, ...}}                                                                                                                                                                                 
E         Use -v to get the full diff                                                                                                                                                                                                       

tests/integration/trading/test_backtest_e2e.py:66: AssertionError 
mhallsmoore commented 3 years ago

Hi @b4d0n3,

Firstly, thank you for building the Dockerfile. Very useful!

Having taken a brief look at the diff between the expected and actual output it seems there are some very small precision differences between the two realised_pnl values (and possibly the others too).

I will investigate the reason for this, but my initial hunch may be differences in the version of Python used by Ubuntu focal (20.04 LTS) and the version I used to generate the test fixture (which is Ubuntu 18.04 LTS).

Thanks once again!

Cheers,

-Mike.

mhallsmoore commented 3 years ago

Hi @b4d0n3,

I've just re-run the current master branch build (without code changes) in TravisCI and while it passes for Python 3.5 and 3.6, it fails for 3.7 and 3.8. This is likely the cause of the problem with the failure in focal.

Digging in a little more I can see that the version of Pandas differs between the two versions. For Python 3.6 we have:

Collecting pandas>=0.25.1
  Downloading pandas-1.1.5-cp36-cp36m-manylinux1_x86_64.whl (9.5 MB)

While for Python 3.7 we have:

Collecting pandas>=0.25.1
  Downloading pandas-1.2.0-cp37-cp37m-manylinux1_x86_64.whl (9.9 MB)

That is, for 3.6 version 1.1.5 of Pandas has been utilised while for 3.7 version 1.2.0 has been chosen. I imagine this is the source of the small discrepancies between the tests.

I will open a separate PR to fix this problem.

Cheers,

-Mike.

b4d0n3 commented 3 years ago

@mhallsmoore I see, thanks for your efforts.

mhallsmoore commented 3 years ago

Hi @b4d0n3,

This has now been fixed and merged by: https://github.com/mhallsmoore/qstrader/pull/350

Cheers,

-Mike.