Skip to content
This repository has been archived by the owner on Feb 15, 2022. It is now read-only.

Simulation stopped working after #1971 #1983

Closed
mmdiego opened this issue Feb 15, 2020 · 7 comments · Fixed by #1987
Closed

Simulation stopped working after #1971 #1983

mmdiego opened this issue Feb 15, 2020 · 7 comments · Fixed by #1987
Labels

Comments

@mmdiego
Copy link
Contributor

mmdiego commented Feb 15, 2020

I've updated my local copy of zenbot after some months and simulation stopped working after this commit.
So I checked out a fresh copy of zenbot and had the same results. If I rolled back to the commit just before #1971, which is ea7929f it starts working again.
I'm testing it using:
zenbot.js sim binance.BNB-USDT --strategy=macd --period=1m --days=1
Maybe it's something related with the exchange?
I'm using node v8.3, but the same happens with v10.

@DeviaVir DeviaVir added the bug label Feb 15, 2020
@jorisw
Copy link
Contributor

jorisw commented Feb 16, 2020

That’s really weird mmdiego, as I’m using the same exchange as you are, also used both Node 8.3 and 10, and the commit you referenced, I wrote in order to fix the problem.

I am running on that commit with a simulator that never stops.

I could only suggest debugging the portion of code that I made changes to in my PR. And I’d be curious to know how many other users chime in with this problem.

@mmdiego
Copy link
Contributor Author

mmdiego commented Feb 17, 2020

I'm trying to debug it, however I´m not an expert in Node.
I've added many prints to see what was happening inside getNext(). I found that it's getting stucked after the second time it's called.

However, I also found that the behavior depends on the "limit" param. I've changed that value to 10 while debugging to simplify the execution. But then I also changed it to 5000 and it started working correctly! However, if I change the simulation to run on BTC-USDT, it started getting stucked again.

I will continue debbugging and I'll post my console output with the prints I've added, but in the meanwhile, can you try your simulation lowering down the limit param?

Thanks!

@jorisw
Copy link
Contributor

jorisw commented Feb 17, 2020

That is very helpful info, mmdiego. I’ll look into it asap.

@james-ingold
Copy link
Contributor

james-ingold commented Feb 18, 2020

Having the same issue, but if I revert to a previous #1971 sim.js, then I get the issue that @jorisw was originally fixing. 😄
I think it will work if you just change this line (294 sim.js) =>
setImmediate(async () => await getNext())
to
return getNext()

@jorisw
Copy link
Contributor

jorisw commented Feb 18, 2020

I think it will work if you just change this line (294 sim.js)

Could you try that, @mmdiego ? I can't get my simulator to fail (yet), with limit 10. And I never quite understood the reason for the setImmediate call there, so I left it in.

@mmdiego
Copy link
Contributor Author

mmdiego commented Feb 18, 2020

Yes, the change proposed seems to fix the problem. It works even with different values of the "limit" param.

@mmdiego
Copy link
Contributor Author

mmdiego commented Feb 23, 2020

I didn't have enough time to test the fix, I've only checked that at least simulations are working. However, I noticed simulations run extremely slow compared to the previous code using setImmediate(). It was also weird that I got slightly different results running the same simulation, so I'm not sure yet if it's the right fix.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants