iCustom giving unexpected values - page 2

New comment
 
Fernando Carreiro #:

I did not say there were errors. I said their were warnings and listed them for you.

Don't code for "human readability". Code for the "machine". Code for efficiency. Only think about the "human" part in the final stages when plotting or outputting to text.

If you have to scale, then scale by the Tick Size or by the Point Size, because those are the units you would use. Don't just scale by some arbitrary number that you think is appropriate.

Also, ATR as the name says, is an average, and averages do not align to exact whole numbers, even if you scale them by the tIck size or point size. You have to be very wary of this when planning your logic.

However in this case, neither the ATR not the decision logic for the ATR is for "humans". It is only for internal "machine" usage. So make the code efficient. Instead, scale the input parameter. You can supply the parameter in units of "points" or "ticks" and then scale it to price range, given that the iATR data will be price range.

I don't yet know if this is the cause of your indicator malfunction or not, but if you eliminate all problematic code from that get go, then there will be less "suspect" code to have to debug to find issues. If you let the "small" things slide, they can easily build up into a cascade effect leaving you not knowing what is going wrong.

My aim is not to find the bugs for you or to debug it. My aim is to help you clean up the code and its logic to the point where you yourself will see the problem more easily.

That being said, with the aim of reaching the first milestone of having the code "flawlessly" compile, with no errors or warnings, I would like you to tackle this first part, of rewriting the  "isATRCrossedUp" function and the handling of the "atrCrossLevel" parameter, so that the code will compile cleanly.

Once that milestone is completed, we will proceed to the next one. Here is my rendition of how I would rewrite this part (using my own style of coding) to resolve the issues mentioned so far. It is not the only solution. It is merely one solution. Each coder will have their own way of looking at things, and mine is just one of many. Please note, that the code still has a "logic bug" in it. I have left it there on purpose so that we can discuss it later. It has to do with the problem of testing for equality between floating point numbers.

My apologies, you did say warning. However, I'm still left not knowing where that warning came from. I don't see it anywhere. I do indeed see the other 2 warnings though.

And I'm all for working it out myself with some guidance. I would much rather learn where my problems are and end up having less of them in the future than to have someone hand me the answer without learning anything. :)

Challenge accepted! I will post back when I'm done.

 

Ok, the decision I made was to keep the code as simple as possible. To that end, I decided to not worry about converting the atr values at all. I understand what they mean and by cutting out conversion it's a little big simpler to look through all the code. Do that end, here is my end result:

//--- Input Variable
input double
   atrCrossLevel = 0.00100;


//--- OnInit
int OnInit(){

   if( atrCrossLevel == 0 ) {
         Print( "Error: Invalid ATR Cross Level" );
         return INIT_PARAMETERS_INCORRECT;
   };
}

//--- Function
bool isATRCrossedUp(double mATRCur, double mATRPrev){
        
   return (mATRCur >= atrCrossLevel) && (mATRPrev < atrCrossLevel);
      
}

All other code is the same and the warnings are gone.

 
Tristen Shaw #: My apologies, you did say warning. However, I'm still left not knowing where that warning came from. I don't see it anywhere.

The first warning comes from when you compile the REX indicator which you use:

Currently the REX indicator has one warning:

declaration of 'weight_sum' hides global variable       MovingAverages.mqh      222     155
 
Tristen Shaw #: Ok, the decision I made was to keep the code as simple as possible. To that end, I decided to not worry about converting the atr values at all. I understand what they mean and by cutting out conversion it's a little big simpler to look through all the code. Do that end, here is my end result: All other code is the same and the warnings are gone.

Do you remember that in one of my previous posts I alerted you about the problems with testing for equality with floating point numbers?

if( atrCrossLevel == 0 )

Well, the fact is that testing for equality with "double" or "float" is almost impossible due to the inherent way in which numbers are converted to and from decimal and binary. Here are some options for your code above, given that ATR is supposed to always be a positive value:

// Option 1:
   if( !( atrCrossLevel > 0 ) )

// Option 2:
   if( atrCrossLevel < DBL_EPSILON )

EDIT: As for "mATRCur >= atrCrossLevel", I will address this later as I will be giving you a different way to detect the cross. Your current logic is prone to fail, so I will show you a more robust way to detect the cross.

Here is some suggested reading:


     
    For the next, step I would like you to repost your new code, and we will then evaluate and look at the OnInit() handler first before looking into the OnCalculate().
     
    Fernando Carreiro #:

    Do you remember that in one of my previous posts I alerted you about the problems with testing for equality with floating point numbers?

    Well, the fact is that testing for equality with "double" or "float" is almost impossible due to the inherent way in which numbers are converted to and from decimal and binary. Here are some options for your code above, given that ATR is supposed to always be a positive value:

    Here is some suggested reading:

    As for the warning, I still don't see it on my logs.

    Metaeditor

    And the MT5 Log:


    EDIT: Oh wait, you are referring to a warning in the REX code itself, not in my code. I didn't code the REX indicator, so I know nothing about it's errors or warnings.
     
    Tristen Shaw #: As for the warning, I still don't see it on my logs. Metaeditor And the MT5 Log:

    I repeat ... The first warning is from when you compile the "REX.mq5" file.

    Forum on trading, automated trading systems and testing trading strategies

    iCustom giving unexpected values

    Fernando Carreiro, 2022.05.15 23:31

    The first warning comes from when you compile the REX indicator which you use:


     

    This is my new code:

    #property copyright "Copyright 2022, MetaQuotes Ltd."
#property link      "https://www.mql5.com"
#property version   "1.00"
#property indicator_chart_window
#property indicator_buffers 5
#property indicator_plots   2

//--- Plot Up
#property indicator_label1  "Buy"
#property indicator_type1   DRAW_ARROW
#property indicator_color1  clrGreen
#property indicator_style1  STYLE_SOLID
#property indicator_width1  5

//--- Plot Down
#property indicator_label2  "Sell"
#property indicator_type2   DRAW_ARROW
#property indicator_color2  clrRed
#property indicator_style2  STYLE_SOLID
#property indicator_width2  5

//--- Input Variables

input int
   atrPeriod = 14,
   confirmPeriod = 14,
   confirmSignal = 14;
   
input double
   atrCrossLevel = 0.00100;
   
input ENUM_MA_METHOD 
   confirmPeriodMethod = MODE_SMA,
   confirmSignalMethod = MODE_SMA;

input bool
   tradeAlert = false;
   
//--- Non-input Variables

bool
   initError = false;
   
int
   barsCalculated = 0;
   
//--- Buffers and Handles

double
   sellBuffer[],
   buyBuffer[],
   atrBuffer[],
   rexPeriodBuffer[],
   rexSignalBuffer[];
   
int
   atrHandle,
   rexHandle;

enum Direction { UNSET, LONG, SHORT };

Direction
   rexTrend = UNSET;

//+------------------------------------------------------------------+
//| Custom indicator initialization function                         |
//+------------------------------------------------------------------+
int OnInit(){
//--- indicator buffers mapping
   
   SetIndexBuffer(0,  sellBuffer,       INDICATOR_DATA);
   SetIndexBuffer(1,  buyBuffer,        INDICATOR_DATA);
   SetIndexBuffer(2,  atrBuffer,        INDICATOR_CALCULATIONS);
   SetIndexBuffer(3,  rexPeriodBuffer,  INDICATOR_CALCULATIONS);
   SetIndexBuffer(4,  rexSignalBuffer,  INDICATOR_CALCULATIONS);
   
   PlotIndexSetDouble(0, PLOT_EMPTY_VALUE, 0.0);
   PlotIndexSetDouble(1, PLOT_EMPTY_VALUE, 0.0);
   
   PlotIndexSetInteger(0, PLOT_ARROW, 159);
   PlotIndexSetInteger(1, PLOT_ARROW, 159);
   
   atrHandle = iATR(Symbol(), Period(),
                    atrPeriod);
                    
   rexHandle = iCustom(Symbol(), Period(), "REX",
                       confirmPeriod,
                       confirmPeriodMethod,
                       confirmSignal,
                       confirmSignalMethod);
                       
   if( !( atrCrossLevel > 0 ) ) {
         Print( "Error: Invalid ATR Cross Level" );
         return INIT_PARAMETERS_INCORRECT;
   };
                       
   if(rexHandle == INVALID_HANDLE || atrHandle == INVALID_HANDLE){
      //--- tell about the failure and output the error code
      PrintFormat( "Failed to create handle of the iCustom indicator for the symbol %s/%s, error code %d",
                  Symbol(),
                  EnumToString(Period()),
                  GetLastError());
      //--- the indicator is stopped early
      initError = true;
      return(INIT_FAILED);
   }
//---
   return(INIT_SUCCEEDED);
}
//+------------------------------------------------------------------+
//| Custom indicator iteration function                              |
//+------------------------------------------------------------------+
int OnCalculate(const int rates_total,
                const int prev_calculated,
                const datetime &time[],
                const double &open[],
                const double &high[],
                const double &low[],
                const double &close[],
                const long &tick_volume[],
                const long &volume[],
                const int &spread[]){
//---
   if(initError || rates_total < 3) return(0);
           
   int
      valuesToCopy,
      calculated = BarsCalculated(atrHandle);
      
   if(calculated <= 0){
      PrintFormat("BarsCalculated returned %d, error code %d", calculated, GetLastError());
      return(0);
   }
   
   valuesToCopy = setValuesToCopy(calculated, prev_calculated, rates_total, barsCalculated);

   if(!fillATRArray(atrBuffer, atrHandle, valuesToCopy))
      return(0);
     
   if(!fillREXArray(rexPeriodBuffer, rexSignalBuffer, rexHandle, valuesToCopy))
      return(0);
      
   barsCalculated = calculated;   
   
   int limit = prev_calculated - 1;
   
   if(prev_calculated == 0){
      buyBuffer[0] =       0.0;
      sellBuffer[0] =      0.0;
      
      limit =              1;
   }
   
   for(int i = limit; i < rates_total; i++){
                                             
      buyBuffer[i] =    0.0;
      sellBuffer[i] =   0.0;
      
      updateREXTrend(rexPeriodBuffer[i], rexSignalBuffer[i]);
      
      if(isATRCrossedUp(atrBuffer[i], atrBuffer[i - 1])){
         
         sellBuffer[i] = rexTrend == LONG ? 0.0 : low[i];
         buyBuffer[i] = rexTrend == LONG ? high[i] : 0.0;
      }
   }
//--- return value of prev_calculated for next call
   return(rates_total);
}
//+------------------------------------------------------------------+

bool isNewBar(){

   static datetime lastTime = 0;
   datetime lastBarTime = (datetime)SeriesInfoInteger(Symbol(), Period(), SERIES_LASTBAR_DATE);

   if(lastTime == 0){
      lastTime = lastBarTime;
      return(false);
   }
   if(lastTime != lastBarTime){
      lastTime = lastBarTime;
      return(true);
   }
   return(false);
}

bool isATRCrossedUp(double mATRCur, double mATRPrev){
        
   return (mATRCur >= atrCrossLevel) && (mATRPrev < atrCrossLevel);
      
}

int setValuesToCopy( int mCalculated,
                     int mPrev_calculated,
                     int mRates_total,
                     int mBarsCalculated){

   if(mPrev_calculated == 0 || mCalculated != mBarsCalculated || mRates_total > mPrev_calculated + 1){
      
      if(mCalculated > mRates_total) return mRates_total;
      else return mCalculated;
      
   } else {
      
      return (mRates_total - mPrev_calculated) + 1;
      
   }

}

bool fillATRArray(double &values_0[], int mATRHandle, int amount){
   ResetLastError();
   
   if(CopyBuffer(mATRHandle, 0, 0, amount, values_0) != amount){
      PrintFormat("Failed to copy data from the ATR indicator, error code %d",GetLastError());
      return(false);
   }
   
   return(true);
}

bool fillREXArray(double &values_0[], double &values_1[], int mREXHandle, int amount){
   ResetLastError();
   
   if(CopyBuffer(mREXHandle, 0, 0, amount, values_0) != amount){
      PrintFormat("Failed to copy data from the REX indicator, error code %d",GetLastError());
      return(false);
   }
   
   if(CopyBuffer(mREXHandle, 1, 0, amount, values_1) != amount){
      PrintFormat("Failed to copy data from the REX indicator, error code %d",GetLastError());
      return(false);
   }
   
   return(true);

}

void updateREXTrend(double mPeriod, double mSignal){

   if( mPeriod > mSignal ){
      rexTrend = LONG;
   }else if ( mPeriod < mSignal ){
      rexTrend = SHORT;
   } else {
      rexTrend = UNSET;
   }

}
     
    Tristen Shaw #: EDIT: Oh wait, you are referring to a warning in the REX code itself, not in my code. I didn't code the REX indicator, so I know nothing about it's errors or warnings.

    That is why I said that I would let that warning slip. Because it is not your code that is at fault. Someone else was not careful about it.

     
    Fernando Carreiro #:

    I repeat ... The first warning is from when you compile the "REX.mq5" file.


    Yeah I figured out what you meant and edited my last post. Apologies.

    I gotcha now, I think we are on the same page.

    12345678
    New comment
    Reason: