tradingstrategy-ai / trade-executor

A Python framework for managing positions and trades in DeFi
https://tradingstrategy.ai
Other
96 stars 28 forks source link

Equity curve and summary stats not in sync #227

Closed AlexTheLion123 closed 1 year ago

AlexTheLion123 commented 1 year ago

See this Discord message

The curve suggests 30-35% return %. Different total profit/value at the end

Image

Image

AlexTheLion123 commented 1 year ago

@Mikko Ohtamaa#5448 Can you help me understand why we rebuild the asset_histories in build_trade_analysis, causing us to kind of duplicate TradingPosition with the TradePosition class and also introduce SpotTrade class?

Because in calculate_summary_statistics, we could loop through self.portfolio.get_all_positions instead of using rebuilt self.get_all_positions, unless I'm missing something.

The only thing I can see is the rebuilding filters out failed trades and does this:

if trade.planned_mid_price not in (0, None):
                price = trade.planned_mid_price
            else:
                # TODO: Legacy trades.
                # mid_price is filled to all latest trades
                price = trade.executed_price
                bad_data_issues = True 

which could maybe be done without rebuilding?

AlexTheLion123 commented 1 year ago

Have opened #259 to move to non-rebuilding approach and remove duplicates

AlexTheLion123 commented 1 year ago

Currently blocked until #259 is merged

AlexTheLion123 commented 1 year ago

Now unblocked, having a look again

AlexTheLion123 commented 1 year ago

Doesn't appear to be an issue anymore, returns appear to now be showing correct value