search

AI4Finance-Foundation / FinRL

FinRL: Financial Reinforcement Learning. 🔥
https://ai4finance.org
MIT License
9.67k stars 2.34k forks source link

FinRL_PaperTrading_Demo - trading operation quantity is 0 #445

Open marcipops opened 2 years ago

marcipops commented 2 years ago

Issue with FinRL_PaperTrading_Demo which gives these results when calling AlpacaPaperTrading(). Quantity should not be 0.

Succesfully add technical indicators Successfully transformed into array 30 Quantity is 0, order of | 0 AAPL sell | not completed. Quantity is 0, order of | 0 CVX sell | not completed. Quantity is 0, order of | 0 INTC sell | not completed. Quantity is 0, order of | 0 CRM sell | not completed. Quantity is 0, order of | 0 VZ sell | not completed. Quantity is 0, order of | 0 AXP buy | not completed. Quantity is 0, order of | 0 CSCO buy | not completed. Quantity is 0, order of | 0 GS buy | not completed. Quantity is 0, order of | 0 KO buy | not completed. Quantity is 0, order of | 0 DOW buy | not completed.

rayrui312 commented 2 years ago

You may need to update the training and testing period. Use longer and newer periods instead of the default ones in the notebook. The agent trade 0 shares because it has never met a similar state before and doesn't know what to do.

marcipops commented 2 years ago

OK - as we discussed earlier on chat, great if you can get the code to have valid defaults in a user parameter block at the top. This will enable the demo code to work 'straight out of the box', and can also be used to check the user environment is all working.

Would trading 0 shares ever be a valid trade? If it doesn't know what to do, would it be best for it not to do anything?

benwaldner commented 2 years ago

I think its not a bug. It just throws amt = 0, since it has no clear signal of buying/ selling. This depends on the market situation. Since its live trading you cant say upfront how the model works on the specific market situation and the used timeframe, ticker etc.

I noticed some times it uses several of those loops to come up with a trade. Otherwise the model must be configured again (training, timeframe etc.). So you get different results.

ghaffari903 commented 2 years ago

I think the step function has a bug:

`def step(self, actions): actions = (actions * self.max_stock).astype(int)

    self.day += 1
    price = self.price_ary[self.day]
    self.stocks_cool_down += 1

    if self.turbulence_bool[self.day] == 0:
        min_action = int(self.max_stock * self.min_stock_rate)  # stock_cd
        for index in np.where(actions < -min_action)[0]:  # sell_index:
            if price[index] > 0:  # Sell only if current asset is > 0
                self.actions_memory.append(actions[index])
                sell_num_shares = min(self.stocks[index], -actions[index])
                self.stocks[index] -= sell_num_shares
                self.amount += (
                    price[index] * sell_num_shares * (1 - self.sell_cost_pct)
                )
                self.stocks_cool_down[index] = 0
        for index in np.where(actions > min_action)[0]:  # buy_index:
            if (
                price[index] > 0
            ):  # Buy only if the price is > 0 (no missing data in this particular date)
                self.actions_memory.append(actions[index])
                buy_num_shares = min(self.amount // price[index], actions[index])
                self.stocks[index] += buy_num_shares
                self.amount -= (
                    price[index] * buy_num_shares * (1 + self.buy_cost_pct)
                )
                self.stocks_cool_down[index] = 0`

to calculate shares both "sell_num_shares" and "buy_num_shares" may be 0, and it has not considered. self.stocks[index] and self.amount // price[index] may be zero,

Athe-kunal commented 2 years ago

That's why there is a flag if price[index]>0. The NaN values are substituted with 0, so it is dealing with NaN values

YangletLiu commented 2 years ago

I thoughts 0 means doing nothing, so do not send the order to the exchange market.

marcipops commented 2 years ago

Agree - please can we get that change done. Also please ensure the quantity ordered respects the lot size in the market trading rules for that specific symbol, otherwise the exchange will return an error. Many thanks

Athe-kunal commented 2 years ago

Currently, this is the way of submitting the order

def submitOrder(self, qty, stock, side, resp):
        if(qty > 0):
          try:
            self.alpaca.submit_order(stock, qty, side, "market", "day")
            print("Market order of | " + str(qty) + " " + stock + " " + side + " | completed.")
            resp.append(True)
          except:
            print("Order of | " + str(qty) + " " + stock + " " + side + " | did not go through.")
            resp.append(False)
        else:
          print("Quantity is 0, order of | " + str(qty) + " " + stock + " " + side + " | not completed.")
          resp.append(True)

Quantity 0 means doing nothing, but it is handled as not completed. So what do you suggest, not to execute anything if it is 0?

marcipops commented 2 years ago

Instructing a broker to order nothing is an invalid operation. On Metatrader for example, the trading terminal returns with an error, as it enforces a minimum lot size. So yes, it makes sense to me to do nothing, than to execute an invalid operation.

marcipops commented 2 years ago

Also the lot size needs to respect the lot step, i.e. minimum increment and normalise the result for the number of digits required. An example of managing the lot size here: https://www.forexfactory.com/thread/184760-how-to-calculate-lot-size (in Metatrader's language MQL)

marcipops commented 1 year ago

Indeed. Sending a trading order to the exchange with quantity 0 is invalid, and will come back with an error. So we should not send a buy or sell order with quantity 0. Not sure why the trading operation component is being told to buy or sell nothing. One would think it should only be instructed to do something valid.

From: Astarag Mohapatra @.> Sent: 09 March 2022 12:59 To: AI4Finance-Foundation/FinRL @.> Cc: marcipops @.>; Author @.> Subject: Re: [AI4Finance-Foundation/FinRL] FinRL_PaperTrading_Demo - trading operation quantity is 0 (Issue #445)

Currently, this is the way of submitting the order def submitOrder(self, qty, stock, side, resp): if(qty > 0): try: self.alpaca.submit_order(stock, qty, side, "market", "day") print("Market order of | " + str(qty) + " " + stock + " " + side + " | completed.") resp.append(True) except: print("Order of | " + str(qty) + " " + stock + " " + side + " | did not go through.") resp.append(False) else: print("Quantity is 0, order of | " + str(qty) + " " + stock + " " + side + " | not completed.") resp.append(True)

Quantity 0 means doing nothing, but it is handled as not completed. So what do you suggest, not to execute anything if it is 0?

- Reply to this email directly, view it on GitHubhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAI4Finance-Foundation%2FFinRL%2Fissues%2F445%23issuecomment-1062895599&data=04%7C01%7C%7Cdf02fe6768a94e6fd7da08da01cc8a73%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637824275296476189%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=IHBaO7GDkrRiN0xFnV3eRVFN9rzgtVxE309sLC17oeg%3D&reserved=0, or unsubscribehttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAG7HSZPVPF5GOMWPRJ3W7TDU7COAFANCNFSM5MWKACVQ&data=04%7C01%7C%7Cdf02fe6768a94e6fd7da08da01cc8a73%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637824275296476189%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=RYOkFilLr%2BkPV7w4yYXq8juQd%2BTPaXMknlv8CdLQjng%3D&reserved=0. Triage notifications on the go with GitHub Mobile for iOShttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.apple.com%2Fapp%2Fapple-store%2Fid1477376905%3Fct%3Dnotification-email%26mt%3D8%26pt%3D524675&data=04%7C01%7C%7Cdf02fe6768a94e6fd7da08da01cc8a73%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637824275296476189%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=r6BmvljzISLNHDXpuuiP5ut4iaFr1UXveH17hAR%2BYFA%3D&reserved=0 or Androidhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.github.android%26referrer%3Dutm_campaign%253Dnotification-email%2526utm_medium%253Demail%2526utm_source%253Dgithub&data=04%7C01%7C%7Cdf02fe6768a94e6fd7da08da01cc8a73%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637824275296476189%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=bkIVyQscEL22paEs9RFCHxMP6Td1bF9jUmB%2B34e7%2Bi8%3D&reserved=0. You are receiving this because you authored the thread.Message ID: @.**@.>>