🐞 Fix bugs introduced by updating Freqtrade about monigomani HOT 18 CLOSED

Updated to freqtrade-2021.11 in: 527b106...fdbb526 🎉

from monigomani.

freqtrade - ERROR - This strategy requires 4800 candles to start. Binance only provides 1000 for 5m.
Any idea why?

Yes, you are on the wrong Freqtrade commit, you currently need an older version of Freqtrade,
as mentioned lately on the front page of the development branch.
Steps for how to change to the recommended specific commit are also on Discord.

It appears TimeFrame-Zoom got broken on newer Freqtrade versions.
It always zooms in now, even when dry/live running, which is not as intended + causing your issues.

This was also mentioned earlier today in the help channel on Discord.

You'll have to wait till I or another willing developer update Freqtrade again and then iron out all the bugs introduced into MGM by doing that.
On my en this will probably happen:

• Right before wrapping up the next MGM release
• Could also happen earlier if I want/need a specific & very recently added feature out of Freqtrade for MGM

from monigomani.

Oh, I didn't notice your note in the readme. Thank you, a fast and precise answer as always :)

from monigomani.

Took a while, but figured out this is the particular commit in freqtrade/freqtrade who is responsible for this issue:

freqtrade/freqtrade@2f33b97

from monigomani.

I've hacked a bit around and it seems the startup_candle_count (2400) during hyperopting isn't accepted anymore. But, the default (400) is.

So I've added a check to only change the startup_candle_count in backtest-mode.

MasterMoniGoManiHyperStrategy.py

358: -            self.startup_candle_count *= self.timeframe_multiplier

358: +            if RunMode(config.get('runmode', RunMode.OTHER)) is RunMode.BACKTEST:
358: +                self.startup_candle_count *= self.timeframe_multiplier

from monigomani.

I've hacked a bit around and it seems the startup_candle_count (2400) during hyperopting isn't accepted anymore. But, the default (400) is.

Yes exactly what I meant! 👏 Would you be up for it to describe this issue in a PR to Freqtrade?

Uhmm, I think so... But my proposal above is an easy patch inside MasterMoniGoManiHyperStrategy.py. That would cost a lot less effort if I'm guessing right?

from monigomani.

Correct. 400

Now that I think more about it, I believe we don't need the multiplier on the startup_candle_count.. 🤔

I found the commit where it got pushed 41f9875
There MoniGoManiHyperStrategy.py file, Line 916: self.startup_candle_count *= self.timeframe_multiplier was added.

I also found HyperOptResults5_Pt1-23-05-2021_MoniGoManiConfiguration_bugfixed_startup_candle_count.log in the Some Test Results/v0.11.0 folder. And I now recall that it was not really an issue back then, but something I believed what was needed for the timeframe zoom implementation to work correctly.

Back then, I thought, if the timeframe zooms in to 5 minutes, then the dataframe timerange will be too short for the larger informative indicators/signals that get added in the dataframe (Because 400*5m startup is shorter then 400*30m startup).

However, if we follow the get_pair_dataframe() function in MasterMoniGoManiHyperStrategy.py, then we end up at Freqtrade's load_pair_history() function, which takes in the startup_candles parameter.

2 things to consider here:

• No matter if under 5m candles or under 30m or any other candles, the 200EMA (which has the longest startup of the signals used by MGM), will always only need 400 candles as it's startup count.
• We are never interested in the indicator/signal data of the backtest_timeframe, only in it's intra-candle price movement

After my re-reflection on it, I believe this line (self.startup_candle_count *= self.timeframe_multiplier) as a whole can be removed safely as the proper fix! 🎉

from monigomani.

@rodrigogs in case you're using docker, I created an image that use freqtrade commit 3503fdb4 as mentioned in Discord. so you only need to change in docker-compose file : from image: freqtradeorg/freqtrade:develop to hoanghnbk/freqtrade_3503fdb4 and it will work

from monigomani.

freqtrade/freqtrade@2f33b97

Good job mate! 🦾 Hmmm this is problematic though... 🤔
The validate_required_startup_candles() function indeed seems to be the issue here 👀
However we cannot modify the execution of this function from at the strategy level I fear..
I believe a small/simple PR to Freqtrade will be needed to resolve this and to allow us to keep using TimeFrame-Zoom.. 🤔
(Just check the RunMode before you execute, an example of how this can be found in the __init__() function in the MasterMoniGoManiHyperStrategy file.)

Because it's ok for Freqtrade to throw this error when you're Dry/Live running, because Binance simply will not allow you to use that many candles during dry/live runs.
But during backtesting/hyperopting this is no issue, because we already downloaded all the candle data prior to our backtesting/hyperopting, so we will not run into Binance API issues.

from monigomani.

I've hacked a bit around and it seems the startup_candle_count (2400) during hyperopting isn't accepted anymore. But, the default (400) is.

Yes exactly what I meant! 👏 Would you be up for it to describe this issue in a PR to Freqtrade?
Perhaps link this issue too, because we're a good example where this leads to issues 😇

from monigomani.

I'll create a PR containing above patch to close this issue. We can discuss it easier if it's needed.

from monigomani.

Uhmm, I think so... But my proposal above is an easy patch inside MasterMoniGoManiHyperStrategy.py. That would cost a lot less effort if I'm guessing right?

Oh damn I didn't notice! Thought you modified Freqtrades source code 😮

from monigomani.

MasterMoniGoManiHyperStrategy.py

358: -            self.startup_candle_count *= self.timeframe_multiplier

358: +            if RunMode(config.get('runmode', RunMode.OTHER)) is RunMode.BACKTEST:
359: +                self.startup_candle_count *= self.timeframe_multiplier

What is the startup candle count that get's printed to the console with this patch? 🤔
If it gets rid of the bug I believe it will be the "unzoomed" 400 candle startup count that is used?

from monigomani.

Correct. 400

from monigomani.

I already did the same thing hahaha

Thanks!

from monigomani.

Even though PR #99 got merged I truly recommend to not update your Freqtrade!

Please remain on Freqtrade commit freqtrade/freqtrade@3503fdb until instructed otherwise!

Only update your Freqtrade if you wish to:

• Check for new breakage in MGM that might occur
• BugFix & PR new breakage if it occurs

from monigomani.

@rodrigogs in case you're using docker, I created an image that use freqtrade commit 3503fdb4 as mentioned in Discord. so you only need to change in docker-compose file : from image: freqtradeorg/freqtrade:develop to hoanghnbk/freqtrade_3503fdb4 and it will work

You also need to update docker/Dockerfile.MoniGoMani

from monigomani.

☑️ Handle Freqtrade's new way of dealing with protections, appears like they aren't used in the .json files anymore

from monigomani.