Is there a way to optimize the following onCalculate method?
Hi there MQL5 experts!,
I recently created a very lightweight indicator (4 lines of code but is a bit slow, however) which takes the SMA from any given symbol and divides by the SMA from any other. It is something like a SymbolA-SymbolB ratio.
My onCalculate's most slow part is the following, this is the part i'd like to optimize:
Then I do this:
The rest of the algorithm is very easy!, it just loops data and calculate the ratio this way, this is the idea:
I know you can optimize onCalculate methods. I've done some research and found the solution in algorithms which calculate themselves the data, a very simple example would be SMA. However I think this is different because I want to take data already calculated from SMA function, and "the problem" here is that the number of rates of any pair of symbols will be always different, in principle.
Can you help me to optimize this idea's algorithm? I need to optimize this part, thks in advance!:
If I understood well, this code:
if(CopyBuffer(MASymbolAHandler,0,0,rates_total,MASymbolABuffer)<=0) return(0);
is executed on each tick, so of course it's very slow as you copy all the data on each tick. You don't have to recalculate your ratio for all bars on each tick.
Anyway, it will be more easy to help you if you post full code.
If I understood well, this code:
is executed on each tick, so of course it's very slow as you copy all the data on each tick. You don't have to recalculate your ratio for all bars on each tick.
Anyway, it will be more easy to help you if you post full code.
Thks, you are very kind, the full code is this, I'd like to keep the idea of relying on other buffers, if possible:
//+------------------------------------------------------------------+ //| GoldSilverRatio.mq5 | //| Copyright © 20013, Laplacianlab - Jordi Bassagañas | //+------------------------------------------------------------------+ #property copyright "Copyright © 2013, Laplacianlab - Jordi Bassagañas" #property link "https://www.mql5.com/en/users/laplacianlab" #property version "1.00" #property description "GoldSilverRatio is intended to work on simple moving averages of both Silver and Gold. You just have to set the SMA parameters and enter the names that your broker puts to Gold and Silver, for instance, XAUUSD and XAGUSD. However, this indicator remains open to be used on other symbols, under your criteria." #property indicator_separate_window #property indicator_buffers 1 #property indicator_plots 1 #property indicator_type1 DRAW_LINE #property indicator_color1 Orange input string Symbol1; // Gold symbol input string Symbol2; // Silver symbol input int MAPeriod=13; // SMA period input int MAShift=0; // SMA shift //--- indicators handlers int MAGoldHandler; int MASilverHandler; //--- indicators buffers double MAGoldBuffer[]; double MASilverBuffer[]; double ratioBuffer[]; //+------------------------------------------------------------------+ //| Custom indicator initialization function | //+------------------------------------------------------------------+ void OnInit() { ArraySetAsSeries(MAGoldBuffer,true); ArraySetAsSeries(MASilverBuffer,true); ArraySetAsSeries(ratioBuffer,true); MAGoldHandler=iMA(Symbol1,_Period,MAPeriod,MAShift,MODE_SMA,PRICE_CLOSE); if(MAGoldHandler==INVALID_HANDLE) Print("Failed to get handle of the iMA Gold"); MASilverHandler=iMA(Symbol2,_Period,MAPeriod,MAShift,MODE_SMA,PRICE_CLOSE); if(MASilverHandler==INVALID_HANDLE) Print("Failed to get handle of the iMA Silver"); SetIndexBuffer(0,ratioBuffer,INDICATOR_DATA); } //+------------------------------------------------------------------+ //| Custom indicator iteration function | //+------------------------------------------------------------------+ int OnCalculate(const int rates_total, const int prev_calculated, const int begin, const double &price[]) { int nGoldBars,nSilverBars; int i,nMinBars,nMaxBars; if(CopyBuffer(MAGoldHandler,0,0,rates_total,MAGoldBuffer)<=0) return(0); if(CopyBuffer(MASilverHandler,0,0,rates_total,MASilverBuffer)<=0) return(0); nGoldBars=ArraySize(MAGoldBuffer); nSilverBars=ArraySize(MASilverBuffer); if(nSilverBars<=nGoldBars) { nMinBars=nSilverBars; nMaxBars=nGoldBars; } else { nMinBars=nGoldBars; nMaxBars=nSilverBars; } ArrayFill(ratioBuffer,0,rates_total,EMPTY_VALUE); for(i=0;i<nMinBars-MAPeriod && !IsStopped(); i++) { ratioBuffer[i]=MAGoldBuffer[i]/MASilverBuffer[i]; } return(rates_total); }
Thks, you are very kind, the full code is this, I'd like to keep the idea of relying on other buffers, if possible:
Here comes the fixed version, how does it look now angevoyageur, it would be great to know your opinion, i think it looks much more professional right now, thks:
//+------------------------------------------------------------------+ //| GoldSilverRatio.mq5 | //| Copyright © 20013, Laplacianlab - Jordi Bassagañas | //+------------------------------------------------------------------+ #property copyright "Copyright © 2013, Laplacianlab - Jordi Bassagañas" #property link "https://www.mql5.com/en/users/laplacianlab" #property version "1.00" #property description "GoldSilverRatio is intended to work on simple moving averages of both Silver and Gold. You just have to set the SMA parameters and enter the names that your broker puts to Gold and Silver, for instance, XAUUSD and XAGUSD. However, this indicator remains open to be used on other symbols, under your criteria." #property indicator_separate_window #property indicator_buffers 1 #property indicator_plots 1 #property indicator_type1 DRAW_LINE #property indicator_color1 Orange input string Symbol1; // Gold symbol input string Symbol2; // Silver symbol input int MAPeriod=13; // SMA period input int MAShift=0; // SMA shift //--- indicators handlers int MAGoldHandler; int MASilverHandler; //--- indicators buffers double MAGoldBuffer[]; double MASilverBuffer[]; double ratioBuffer[]; //+------------------------------------------------------------------+ //| Custom indicator initialization function | //+------------------------------------------------------------------+ void OnInit() { ArraySetAsSeries(MAGoldBuffer,true); ArraySetAsSeries(MASilverBuffer,true); ArraySetAsSeries(ratioBuffer,true); MAGoldHandler=iMA(Symbol1,_Period,MAPeriod,MAShift,MODE_SMA,PRICE_CLOSE); if(MAGoldHandler==INVALID_HANDLE) Print("Failed to get handle of the iMA Gold"); MASilverHandler=iMA(Symbol2,_Period,MAPeriod,MAShift,MODE_SMA,PRICE_CLOSE); if(MASilverHandler==INVALID_HANDLE) Print("Failed to get handle of the iMA Silver"); SetIndexBuffer(0,ratioBuffer,INDICATOR_DATA); Print("GoldSilverRatio OnInit has been run..."); } //+------------------------------------------------------------------+ //| Custom indicator iteration function | //+------------------------------------------------------------------+ int OnCalculate(const int rates_total, const int prev_calculated, const int begin, const double &price[]) { int nGoldBars,nSilverBars; int i,nMinBars,nMaxBars; //--- running onCalculate for the first time if(prev_calculated>rates_total || prev_calculated<=0) { if(CopyBuffer(MAGoldHandler,0,0,rates_total,MAGoldBuffer)<=0) return(0); if(CopyBuffer(MASilverHandler,0,0,rates_total,MASilverBuffer)<=0) return(0); nGoldBars=ArraySize(MAGoldBuffer); nSilverBars=ArraySize(MASilverBuffer); if(nSilverBars<=nGoldBars) { nMinBars=nSilverBars; nMaxBars=nGoldBars; } else { nMinBars=nGoldBars; nMaxBars=nSilverBars; } ArrayFill(ratioBuffer,0,rates_total,EMPTY_VALUE); for(i=0;i<nMinBars-MAPeriod && !IsStopped(); i++) { ratioBuffer[i]=MAGoldBuffer[i]/MASilverBuffer[i]; } } //--- running onCalculate when arriving new data else { if(CopyBuffer(MAGoldHandler,0,0,1,MAGoldBuffer)<=0) return(0); if(CopyBuffer(MASilverHandler,0,0,1,MASilverBuffer)<=0) return(0); ratioBuffer[0]=MAGoldBuffer[0]/MASilverBuffer[0]; } return(rates_total); }
Here comes the fixed version, how does it look now angevoyageur, it would be great to know your opinion, i think it looks much more professional right now, thks:
Your new version fix the issue of speed, but there is still another problem, which is synchronisation of data for both symbols.
Your first version is repainting (sometimes), your second version don't always calculated right. You have to be sure that index 0 of your buffers contain data for same candle (same opening time). There can be missing candle, or current candle (index=0) can be shifted by 1.
I hope I am clear enough, I will post my version later.
Edit :
As a side note, the documentation says :
Note
After binding, the dynamic array buffer[] will be indexed as in common arrays, even if the indexing of timeseries is pre-installed for the bound array. If you want to change the order of access to elements of the indicator array, use the ArraySetAsSeries() function after binding the array using the SetIndexBuffer() function. Please note that you can't change the size for dynamic arrays set as indicator buffers by the function SetIndexBuffer(). For indicator buffers, all operations of size changes are performed by the executing sub-system of the terminal.
So this line
ArraySetAsSeries(ratioBuffer,true);
should be placed after
SetIndexBuffer(0,ratioBuffer,INDICATOR_DATA);
but it seems that doesn't affect the results, so maybe the documentation is obsolete ?
Your new version fix the issue of speed, but there is still another problem, which is synchronisation of data for both symbols.
Your first version is repainting (sometimes), your second version don't always calculated right. You have to be sure that index 0 of your buffers contain data for same candle (same opening time). There can be missing candle, or current candle (index=0) can be shifted by 1.
I hope I am clear enough, I will post my version later.
Edit :
As a side note, the documentation says :
So this line
should be placed after
but it seems that doesn't affect the results, so maybe the documentation is obsolete ?
Ok, I think that both graphics are ok now, aren't they? Anyway I think the sync problem is solved by that algorithm maybe for that reason it's a bit slow when calculating both buffers for the first time. Thks very much!
Ok, I think that both graphics are ok now, aren't they? Anyway I think the sync problem is solved by that algorithm maybe for that reason it's a bit slow when calculating both buffers for the first time. Thks very much!
Not. Your last version is bugged, in that way the data for both symbols aren't synchronized.
Say for example you place your indicator on first symbol (XAUUSD), so OnCalculate() is called on each tick. When a new candle arises for XAUUSD (candle with index 0) how do you know there is also a new candle on other symbol ? If there is not, your data aren't synchronized, your code is :
if(CopyBuffer(MAGoldHandler,0,0,1,MAGoldBuffer)<=0) return(0); if(CopyBuffer(MASilverHandler,0,0,1,MASilverBuffer)<=0) return(0); ratioBuffer[0]=MAGoldBuffer[0]/MASilverBuffer[0];
so you are working with candle 0 for both symbol, but candle 0 for XAUUSD can be different candle that one of other symbol. Your value of ratioBuffer[0] is then wrong, if there is no other tick on that candle, it will remain wrong. And what if a symbol suddenly don't have a candle ?
Not. Your last version is bugged, in that way the data for both symbols aren't synchronized.
Say for example you place your indicator on first symbol (XAUUSD), so OnCalculate() is called on each tick. When a new candle arises for XAUUSD (candle with index 0) how do you know there is also a new candle on other symbol ? If there is not, your data aren't synchronized, your code is :
so you are working with candle 0 for both symbol, but candle 0 for XAUUSD can be different candle that one of other symbol. Your value of ratioBuffer[0] is then wrong, if there is no other tick on that candle, it will remain wrong. And what if a symbol suddenly don't have a candle ?
Thks for your feedback. So you mean the first version is ok?
On the other hand, I assume that this part of algorithm
if(CopyBuffer(MAGoldHandler,0,0,1,MAGoldBuffer)<=0) return(0); if(CopyBuffer(MASilverHandler,0,0,1,MASilverBuffer)<=0) return(0); ratioBuffer[0]=MAGoldBuffer[0]/MASilverBuffer[0];
always takes last data. I mean, those 2 CopyBuffers are always taking the last info that arrived from the server. What's wrong with my assumptions? Why you say that? MASilverBuffer[0] and MAGoldBuffer[0] have exactly the same info that in the previous version!
Thanks again for your help.
Thks for your feedback. So you mean the first version is ok?
On the other hand, I assume that this part of algorithm
always takes last data. I mean, those 2 CopyBuffers are always taking the last info that arrived from the server. What's wrong with my assumptions? Why you say that? MASilverBuffer[0] and MAGoldBuffer[0] have exactly the same info that in the previous version!
Thanks again for your help.
I will explain my point with more details when I have some time...be patient.
I will explain my point with more details when I have some time...be patient.
- www.mql5.com
- Free trading apps
- Over 8,000 signals for copying
- Economic news for exploring financial markets
You agree to website policy and terms of use
Hi there MQL5 experts!,
I recently created a very lightweight indicator (4 lines of code but is a bit slow, however) which takes the SMA from any given symbol and divides by the SMA from any other. It is something like a SymbolA-SymbolB ratio.
My onCalculate's most slow part is the following, this is the part i'd like to optimize:
Then I do this:
The rest of the algorithm is very easy!, it just loops data and calculate the ratio this way, this is the idea:
I know you can optimize onCalculate methods. I've done some research and found the solution in algorithms which calculate themselves the data, a very simple example would be SMA. However I think this is different because I want to take data already calculated from SMA function, and "the problem" here is that the number of rates of any pair of symbols will be always different, in principle.
Can you help me to optimize this idea's algorithm? I need to optimize this part, thks in advance!: