hootnot / oandapyV20-examples

Examples demonstrating the use of oandapyV20 (oanda-api-v20)
MIT License
147 stars 65 forks source link

Unable to fill SHORT orders #13

Closed Pearce94 closed 6 years ago

Pearce94 commented 6 years ago

Using simplebot, it executes appropriate long trades; however when the state switches from LONG to SHORT the order gets placed and immediately canceled. Is there anyway to correct this?

"orderCreateTransaction": {
    "batchID": "604", 
    "positionFill": "DEFAULT", 
    "stopLossOnFill": {
      "timeInForce": "GTC", 
      "price": "1.19335"
    }, 
    "userID": ......, 
    "takeProfitOnFill": {
      "timeInForce": "GTC", 
      "price": "1.19293"
    }, 
    "timeInForce": "FOK", 
    "instrument": "EUR_USD", 
    "reason": "CLIENT_ORDER", 
    "requestID": "42343179345476034", 
    "time": "2017-09-15T09:01:02.427915503Z", 
    "units": "1000000", 
    "type": "MARKET_ORDER", 
    "id": "604", 
    "accountID": "....."
  }, 
  "orderCancelTransaction": {
    "orderID": "604", 
    "userID": ......, 
    "batchID": "604", 
    "reason": "TAKE_PROFIT_ON_FILL_LOSS", 
    "requestID": "42343179345476034", 
    "time": "2017-09-15T09:01:02.427915503Z", 
    "type": "ORDER_CANCEL", 
    "id": "605", 
    "accountID": "......"
  }, 
hootnot commented 6 years ago

did you enter the units as negative ? If not, you placed a long order, stops are on 'the other side' so things got differently then. I'm pretty sure you passed a possitive number for units.

Pearce94 commented 6 years ago

I did enter a positive number for units, however I tried with negative and it came back with the same result. Forgive me if I'm wrong, but should this not change units to positive or negative based on SHORT/LONG?

def _botstate(self):
        # overall state, in this case the state of the only indicator ...
        prev = self.state
        self.state = self.indicators[0].state
        units = self.units
        if self.state != prev and self.state in [SHORT, LONG]:
            logger.info("state change: from %s to %s", mapstate(prev),
                        mapstate(self.state))
            units *= (1 if self.state == LONG else -1)
            self.close()
            self.order()

And then apply either the positive or negative units to the direction to determine TP /SL?

direction = 1 if self.units > 0 else -1
        if self.clargs.takeProfit:   # takeProfit specified? add it
            tpPrice = self.pt._c[self.pt.idx-1] * \
                      (1.0 + (self.clargs.takeProfit/100.0) * direction)
            mop.update({"takeProfitOnFill":
                        TakeProfitDetails(price=frmt(tpPrice)).data})

        if self.clargs.stopLoss:     # stopLosss specified? add it
            slPrice = self.pt._c[self.pt.idx-1] * \
                      (1.0 + (self.clargs.stopLoss/100.0) * -direction)
            mop.update({"stopLossOnFill":
                        StopLossDetails(price=frmt(slPrice)).data})

I think what may be happening is that it is setting it as a long trade but having the TP below the SL (as in a short trade), and is therefore immediately cancelling.

hootnot commented 6 years ago

make sure you have SL and TP on the right side. It is easy to check that by logging the values. If you enter fixed prices, say: for LONG: SL 1.18 TP 1.21 and SHORT SL 1.20 TP 1.18 you should have no errors with the current price. These are simply to add by just adding:

mop.update({"stopLossOnFill":
                        StopLossDetails(price=frmt(slPrice)).data})
# override the previous just to test
mop.update("stopLossOnFill": StopLossDetails(price=1.20).data})

and do so for the other values. Order should be accepted then.

Pearce94 commented 6 years ago

I did what you suggested, and it worked for short trades, but then the same problem as before rose for long trades. I tried this lengthly workaround:

 if self.state==LONG:
        if self.clargs.takeProfit:   # takeProfit specified? add it
            tpPrice = self.pt._c[self.pt.idx-1] * \
                      (1.0 + (self.clargs.takeProfit/100.0) )
            mop.update({"takeProfitOnFill":
                        TakeProfitDetails(price=frmt(tpPrice)).data})

        if self.clargs.stopLoss:     # stopLosss specified? add it
            slPrice = self.pt._c[self.pt.idx-1] * \
                      (1.0 + (self.clargs.stopLoss/100.0) *(-1))
            mop.update({"stopLossOnFill":
                        StopLossDetails(price=frmt(slPrice)).data})
  elif self.state==SHORT:   
        if self.clargs.takeProfit:   # takeProfit specified? add it
            tpPrice = self.pt._c[self.pt.idx-1] * \
                      (1.0 + (self.clargs.takeProfit/100.0) * (-1))
            mop.update({"takeProfitOnFill":
                        TakeProfitDetails(price=frmt(tpPrice)).data})

        if self.clargs.stopLoss:     # stopLosss specified? add it
            slPrice = self.pt._c[self.pt.idx-1] * \
                      (1.0 + (self.clargs.stopLoss/100.0) )
            mop.update({"stopLossOnFill":
                        StopLossDetails(price=frmt(slPrice)).data})

For short trades, it now actually enters the trade with correct TP/SL before cancelling order as soon as its in.

hootnot commented 6 years ago

Sorry, I realize (a bit late) that you use the example from the repo I will have a closer look. I will come back to it.

Pearce94 commented 6 years ago

Okay great, thanks.

hootnot commented 6 years ago

@Pearce94 : I've noticed a flaw in the way units were processed

The _botstate() method modifies the units to a negative value. But units is local there so it's value gets lost and the self.units, which stays always positive, is used in the order() method.

I've changed that. So order() now gets the units as parameter and it will be negative in case of a short.

I also saw that there was a static value for the instrument. Changed that too.

If you run:

ython simplebot.py --longMA 5 --shortMA 2 --stopLoss 0.1 --takeProfit 0.2 --instrument EUR_USD  --gran M1 --units 1000

things should function properly. I just ran a short test with these short averages and it seemed OK. If not please let me know. Code is in the simplebot-fix branch. If you have no further issues I will merge it. So pls let me know.

Pearce94 commented 6 years ago

Seems to be working, thanks!