Errors, bugs, questions - page 105

 
Interesting:

The tester has its own separate list of tools and needs to be generated (preferably done when initialising the EA).

Thank you very much! Got it. Missed a lot...
 
Renat:

The code was approximate (copied from two pieces), but your comments are correct.

Here is the corrected version:

I've added a couple of Print() calls to it to make it clear how it actually works:

double CalculateMaxVolume(string symbol)
  {
   double price=0.0;
   double margin=0.0;
//--- select lot size
   if(!SymbolInfoDouble(symbol,SYMBOL_ASK,price))                return(0.0);
   if(!OrderCalcMargin(ORDER_TYPE_BUY,symbol,1.0,price,margin)) return(0.0);
   if(margin<=0.0)                                            return(0.0);

   double lot_pure=AccountInfoDouble(ACCOUNT_FREEMARGIN)/margin;
   double lot=NormalizeDouble(lot_pure,2);
   Print("lot_pure = ", lot_pure, ", lot = ", lot);
//--- normalize and check limits
   double stepvol=SymbolInfoDouble(symbol,SYMBOL_VOLUME_STEP);
   if(stepvol>0.0)
     {
      double newlot=stepvol*NormalizeDouble(lot/stepvol,0);
      if(newlot>lot) { Print("Чёрт побери: lot = ", lot, ", newlot = ", newlot);
                       lot=NormalizeDouble(newlot-stepvol,2);
                     }
      else           lot=newlot;
     }

   double minvol=SymbolInfoDouble(symbol,SYMBOL_VOLUME_MIN);
   if(lot<minvol) lot=0.0;   // 

   double maxvol=SymbolInfoDouble(symbol,SYMBOL_VOLUME_MAX);
   if(lot>maxvol) lot=maxvol;
//--- return trading volume
   return(lot);
  }

void OnStart()
{
  Print("CalculateMaxVolume(Symbol()) = ", CalculateMaxVolume(Symbol()));
}

/* Вывод в лог (хронология - сверху вниз)
CO      0       1 (EURUSD,M15)  01:40:33        lot_pure = 7.799703611262773, lot = 7.8
JG      0       1 (EURUSD,M15)  01:40:33        Чёрт побери: lot = 7.8, newlot = 7.800000000000001
MQ      0       1 (EURUSD,M15)  01:40:33        CalculateMaxVolume(Symbol()) = 7.7
*/

It works correctly now. But with a caveat - under the conditions that are actually encountered now. If they are extended in the future, this code will start making mistakes in some cases under certain conditions. I will explain below.

Actually I had to make some research and got very interesting results as a result. But let's speak about them one by one.

The first thing that jumps out at me is this: why did I have to dance so much with NormalizeDouble()? Well, what does NormalizeDoubel() do? Binds a free value to the grid. For example, in this code fragment:

double lot=NormalizeDouble(lot_pure,2);

NormalizeDouble() takes as a parameter lot_pure (free value, i.e. that calculated by free margin and necessary margin for 1 lot without any rounding and other binding) and gives the value, bound to the nearest value of the grid with start at 0 and step 0.01.
Here it is important to note: to the nearest node of the grid, including the larger one!

What is this binding for in this place of the code? And why to the 0.01 grid and not to, say, 0.001?

By the way, we can see that the result marked with CO in the log (the first line of the shown fragment) has resulted in the increased value.

Further, we know that all trade functions that accept the number of lots as one of parameters, require that this value is bound to the grid: minvol + N * stepvol, where N is an integer from 0 to the value of an integer part of the expression (maxvol - minvol) / stepvol. Accordingly, the free lot value obtained in this fragment:

double lot_pure=AccountInfoDouble(ACCOUNT_FREEMARGIN)/margin;

must be bound to the nearest node of the specified grid: minvol + N * stepvol. This means that you must first subtract minvol from the value of lot before dividing by stepvol (to get that integer N), and after multiplying by N, add minvol. But you immediately divide by stepvol, implicitly assuming that stepvol is the divisor of minvol, i.e. it fits an integer number of times, because only if this condition is met can you "simplify" in this way and not get side effects:

double newlot=stepvol*NormalizeDouble(lot/stepvol,0);

Again you use NormalizeDouble(), this time to bind to a grid with start at 0 and step 1, i.e. to integers. The mesh binding parameters are correct, but the snap tool is a bit unfortunate: it binds to the nearest mesh node, including a larger one if it happens to be closer. And in our case it will lead to subsequent compulsory working out of correction code. Why not use here a wonderful tool of binding to "a grid of integers" using instead of calling NormalizeDouble() a cast to integer type which does not increment the value being cast, but only decrement it to the nearest integer if necessary, i.e. - what we need?

But one more interesting artifact occurs here which is proved in the second line of the cited fragment marked with JG. It turns out that the "0.1 * NormalizeDouble(7.8 / 0.1)" expression gives the result 7.800000000000001, which causes the correcting code to work! Why would you need such a code that performs so poorly that you need to add a correcting one to it?

Clearly, the code of binding to the grid of acceptable lot values must be replaced with a better one.

Of course, we can leave this code as well - after all, the correcting part of the code, if anything, will work. Here is the third line of the log proves it: the result returns right at the end. But this code, on the other hand, is an indicator of its creators' professionalism and quality. Including the quality of the code of the MT5 platform. And proof of this will be given by me, because I have stumbled on two bugs as a result of my research.

By the way, let's take another look at the code of initial binding of the calculated lot_pure value and the correcting code:

double lot=NormalizeDouble(lot_pure,2);
...
lot=NormalizeDouble(newlot-stepvol,2);
In both cases there is a binding to the grid with the step 0.01. Why this grid? Because you want to bind to the minvol + N * stepvol grid that has a stepvol. What will happen when in the future we will have both a minimum lot value and a stepvol of 0.001?

It is very simple - the code will give wrong results in cases when in the process of binding to the grid with the step 0.01 the free value changes on the value more than 0.001. This is the caveat I mentioned at the beginning.

If "rounding up" by more than 0.001, lot values will be returned with insufficient free margin to open the position, while in case of "rounding down" by the same amount - understated values or 0, if the free value is within the range of 0.001 - 0.004999...

That is, the code contains a potential bug for the future. It is a question of developer professionalism and quality of their code.

Now, taking into account what I have found, I will suggest my own variant of the function:

double CalculateMaxVolume_New(string symbol)
{
  double stepvol = SymbolInfoDouble(symbol, SYMBOL_VOLUME_STEP);
  double minvol  = SymbolInfoDouble(symbol, SYMBOL_VOLUME_MIN);
  double maxvol  = SymbolInfoDouble(symbol, SYMBOL_VOLUME_MAX);

  // Не доверяем значениям, которые вернули функции
  if(stepvol > 0 && minvol > 0 && maxvol > minvol)
  {
    double tmp = 0;

    // Вычисляем цену Ask, Margin на 1 лот, лотов на FreeMargin
    if(SymbolInfoDouble(symbol, SYMBOL_ASK, tmp)            && tmp > 0       &&
       OrderCalcMargin(ORDER_TYPE_BUY, symbol, 1, tmp, tmp) && tmp > 0       &&
       (tmp = AccountInfoDouble(ACCOUNT_FREEMARGIN) / tmp)         >= minvol &&
       tmp < ULONG_MAX * stepvol)
    {
      Print("pure_lot = ", tmp); // Эту строку нужно удалить из рабочего кода

      if(tmp > maxvol) // Здесь в tmp содержится недискретизированное число лотов
        return maxvol;

      // Привязываемся к сетке дискретизации
      return minvol + stepvol * (ulong)((tmp - minvol) / stepvol);
    }
  }

  return 0;
}

void OnStart()
{
  Print("CalculateMaxVolume_New(Symbol()) = ", CalculateMaxVolume_New(Symbol()));
}

/* Вывод в лог (хронология - сверху вниз)
LQ      0       1 (EURUSD,M15)  01:39:07        pure_lot = 7.799095304944626
KD      0       1 (EURUSD,M15)  01:39:07        CalculateMaxVolume_New(Symbol()) = 7.7
*/

There are several cases, in relation to the lots value (stored in my tmp), calculated but not yet bound to a grid of valid values. Let's call the grid-bound value discretized.

1. The case when tmp < minvol. In this case, the inequality will also remain after tmp is discretized, because the discretization process only involves the reduction of the calculated value (otherwise there is not enough free margin, for the calculated value is the maximum possible value for the given amount of free margin).

Therefore this case can be eliminated at an early stage, before sampling.

2. The case when tmp > maxvol. In this case the limitation is not free margin, but the maximum allowed number of lots accepted by the trading functions. In this case, just return the value of maxvol.

No sampling of tmp is required to simply return the value of maxvol, so this case is also cut off before the sampling code.

3. The case when minvol <= tmp <= maxvol. In this case it is necessary to discretize, but the discretized value remains within an inequality for this case, i.e. there is no need to correct anything after discretization.

The sampling code is simple and efficient:

return minvol + stepvol * (ulong)((tmp - minvol) / stepvol);

Here, the expression "(tmp - minvol) / stepvol" calculates the same number N (the anchor grid parameter), but with a fractional part. Since the inequality minvol <= tmp (case 3) is fulfilled here, it is thereby guaranteed that the calculated value is non-negative. Then we explicitly cast the calculated value to a value of ulong type. It is ulong because it's guaranteed that the calculated value is non-negative.

During conversion to integer type, the fractional part of the real type is discarded. It is not rounding to the nearest one, but the fractional part is discarded, which guarantees that the maximum value of lots, which free margin allows, will not be increased. That is exactly what we need.

Since the integer value of N is obtained, then the maximum discretized value of the number of lots, which allows the free margin, is obtained in the standard way (i.e. the next integer in the grid of discretized values of lots will not allow the free margin, while the obtained N will still allow the free margin): "minvol + stepvol * N".

I want to highlight one quite important point. The thing is that numbers of double type can take maximum values up to about 1.8e308, while numbers of ulong type - only about 1.8e19 (it's just an entry for convenience, the constant 1.8e19 is not of ulong type).

What if the value of the "(tmp - minvol) / stepvol" expression exceeds 1.8e19? In this case, during conversion to the ulong type, "cutting" will occur - the value will be the remainder of the integer division of the expression value by ULONG_MAX.

If the numbers were not so large, in terms of MQL5, it would look as "X % ULONG_MAX", where X is an integer equal to "(tmp - minvol) / stepvol".

This is not a typical case, but why leave bugs in the code? Moreover, the MQL5 library functions cannot be trusted, they may return any nonsense (I'll give proof of that).

For cases where the value of "tmp / stepvol" expression does not fit in 1.8e19, we purposely introduced a check (the last line of if condition):

tmp < ULONG_MAX * stepvol

Of course, you can write "tmp / stepvol < (double)ULONG_MAX" but first, I try to avoid division operations when there is no explicit necessity and second, I would have to perform an explicit type conversion since the ULONG_MAX constant is of ulong type, When comparing operands, they are not implicitly cast to a higher type (at least it's so in C/C++), and in the third - I would not have encountered a nice bug in the language itself, not even in library functions - the bug is not that in DNA, but literally in molecules and atoms of MQL5.

The left operand of the comparison operation is tmp and has the double type while the right operand is the "ULONG_MAX * stepvol" expression and has the double type too.

This expression has two operands, one of type ulong and the other of type double. According to the rules of implicit type conversion, first the operand of the lower type is cast to the operand of the higher type, the operation is executed and the result is cast to the operand of the higher type. The double type is "older" than ulong, that's why the ULONG_MAX value of ulong is implicitly cast to double, the operation is performed and the result is of double type.

However, there is a bug here which, by the way, appears not always but only in some cases including this one, and the bug consists in that the result of the "ULONG_MAX * stepvol" expression is only stepvol's value.

Therefore, the function I'm displaying doesn't work and won't work until MetaQuotes developers fix this bug.

In order to start using this function now, you should take advantage of the bug's peculiarity: it disappears if you perform explicit type conversion:

tmp < (double)ULONG_MAX * stepvol

Returning to the described check: it guarantees that the value of the "tmp / stepvol" expression will not exceed ULONG_MAX. But the sampling code uses the expression "(tmp - minvol) / stepvol".

The value of this expression will also not exceed ULONG_MAX because the previous checks ensure that minvol > 0 and tmp >= minvol, i.e. tmp - minvol < tmp.

Therefore, the guarantee of not exceeding ULOMG_MAX also applies to the expression "(tmp - minvol) / stepvol".

Generally, one of the major differences between a professional and a layman is that a professional can at least guarantee something, while a layman...

I have disassembled both found bugs in the otherpost, at the same time clarifying what MetaQuotes did and did not manage to do.

 

Для чего в этом месте кода выполняется эта привязка? И почему именно к сетке 0.01, а не к, скажем, 0.001?

In the system, the minimum lot = 0.01


Notes:

  1. Your initial condition minvol + N * stepvol is not guaranteed to be correct, you can set minlot to a different value and your logic will be broken.
  2. You should not have switched to ulong - you created difficulties for yourself, and then wrote a whole page of thoughts about it
  3. Substitution of tmp in your code is too clever, my version is much clearer in terms of operations
 

I speak only for myself (but if you see your reflection, you're not the only one).

Over the past few months of the bug chase, I have developed the habit of first considering a non-working program as a bug in MetaTrader.

Why so, it's just a well-tested pattern, if something does not work, then it is a bug and let the alarm bells ring.

Example: I found a bug, sent a request to servicedesk, they wrote a verification code, but nothing.

I applied again and in anticipation of an answer found my clumsiness.

The result is that I'm ashamed of having distracted people on the spot.

But analyzing the flow of messages I understand that the mass of people, even if smart people are still subjected to the psychology of the crowd.

If there are bugs, I will write a bug and let Renat sort out my code and point the finger at my error.

I understand that tolerance does not allow to say: yes you are a moron, your code is crooked.

But you can't go that far, and prolonging it further all staff MQ will soon be engaged in that sit on other people's codes in mournful contemplation "but why do we need it all", while the championship is coming, and there and go to the real accounts are not far off.

I'll wrap up, my motto for today is "If you're going to publish a bug, check whether the problem is in your hands".

Общайтесь с разработчиками через Сервисдеск!
Общайтесь с разработчиками через Сервисдеск!
  • www.mql5.com
Ваше сообщение сразу станет доступно нашим отделам тестирования, технической поддержки и разработчикам торговой платформы.
 

New build - new problems. Expert, which worked fine in 306 after compilation in 314 (compilation without errors), gives out in tester:

2010.08.21 17:03:36 Core 1 disconnected
2010.08.21 17:03:36 Core 1 tester stopped because OnInit failed
2010.08.21 17:03:36 Core 1 2010.01.04 00:00:00 Access violation read to 0x0000000000000014
2010.08.21 17:03:36 Core 1 2010.01.04 00:00:00 Balance=10000.00 Equite=10000.00 Profit=0.00
2010.08.21 17:03:36 Core 1 2010.01.04 01.04 00:00:00 PriceChannel_multi_Ch_Timer Expert Advisor started working in 2010.01.04 00:00 on EURUSD chart for H1 period

It also unloads in real life. It appeared that the source of error is a line

m_symbol[j].Name(TradeSymbols[i]);

Replacing it with a couple of lines

string curSymbol=TradeSymbols[i];
m_symbol[j].Name(curSymbol);

returned the status quo to the Expert Advisor.

What is the problem?

By the way, code compiled in last build, works fine in this one too.

 
Valmars:

What's the matter ???

By the way, the code compiled in the last build works fine in this one as well.

Our mistake - we will definitely fix it.
 
Renat:

Minimum lot = 0.01


Notes:

  1. Your initial condition minvol + N * stepvol is not guaranteed to be correct, you can set minlot to a different value and your logic will be broken.
  2. You should not have switched to ulong - you created difficulties for yourself, and then wrote a whole page of thoughts about it
  3. Substitution of tmp in your code is too smart, while my version is much clearer in terms of operations.

Right now the system's minimum lot = 0.01. But what about in a year? In two years?

1) Which condition is correct? What then is the correct formula? Say, for minvol = 0.15 and stepvol = 0.1 - what would be the first few valid lot values? a) 0.15, 0.25, 0.35... ? б) 0.15, 0.2, 0.3... ? в) ... ? I have been assuming variant a.

2. I switched to ulong for a reason - I have the right to choose the type with the widest range, so that it would be enough for the widest possible range of cases, because such functions are very basic bricks. And the fact that I have come across a bug does not mean that it was me who created the problems. :) Reasoning wrote more for others to make it clear to the widest possible range - we have here not a personal correspondence.

3. Substitution is not tricky - just saving, so as not to create variables of one-time use. And it was checked and verified that the variable is passed by reference when a function is called at most once, so that possible errors are avoided because of it. If it bothers some of us, we can create a variable for each transferred value (even an intermediate one, such as Ask price), as you have done. This point is not important.

Much more important is the mechanism of binding to the grid of admissible values, which, moreover, doesn't require correction, and which guarantees against the occurrence of glitches in different not very typical cases, while maintaining the maximum possible simplicity.

The premise is that the basic building block should be as robust and versatile as possible - then the whole house will probably even survive an earthquake.

 
Urain:

I speak only for myself (but if you see your reflection, you're not the only one).

Over the past few months of the race for bugs, I have developed the habit of considering a bug in MetaTrader as a bug in the first place.

Why so, just a well established pattern, if something does not work, then it is a bug and let the alarm bells ring.

The fact that MQL5 and MQL5 bugs look a lot like theirs plays an important role here. And there are a lot of MQL5 bugs.

If MQL5 had significantly fewer bugs and they weren't so simple, it would be much harder to confuse them.

Urain:

They have already started to think about the possibility to start the Championship, and the time has come for them to start working on the real trading accounts.

I will wrap it up, today's motto is "If you're going to publish a bug, check whether the problem is in your hands".

The fact that the Championship for Expert Advisors written ONLY in MQL5 is a gamble was clear at the time the decision was announced. But the EA management has its own vision. They have decided so themselves. No one interfered with their decision. So, so what if the championship is around the corner, they made a life for themselves.

Here it's easy: you need to do some work on localization of the bug: start removing from the code everything that doesn't affect the bug. Finally you'll get a kind of test example, which is small enough but demonstrates mostly the bug. This will no longer be "someone else's code" but "code that demonstrates bug MQL5".

 
Wrote a script to test the OrderCalcMargin() function
void OnStart()
  {
//---
   int total=SymbolsTotal(false);
   double marginbay;
   double marginsell;
   MqlTick last_tick;
   for(int i=0;i<total;i++)
     {

      string symbol=SymbolName(i,false);
      Print("************************************************");
      Print("Инструмент - ",symbol);
      Print("Валюта депозита = ",AccountInfoString(ACCOUNT_CURRENCY));
      Print("Базовая валюта = ",SymbolInfoString(symbol,SYMBOL_CURRENCY_BASE));
      Print("Валюта маржи = ",SymbolInfoString(symbol,SYMBOL_CURRENCY_MARGIN));
      if(SymbolInfoTick(symbol,last_tick))
        {
         OrderCalcMargin(ORDER_TYPE_BUY,symbol,1.0,last_tick.ask,marginbay);
         OrderCalcMargin(ORDER_TYPE_SELL,symbol,1.0,last_tick.bid,marginsell);
         Print("Маржа для покупки = ",marginbay);
         Print("Маржа для продажи = ",marginsell);
        }
      else Print("SymbolInfoTick() failed, error = ",GetLastError());
     }
  }
//+------------------------------------------------------------------+
The function returns zero for some instruments, is this a bug or is it designed that way?
 
sergey1294:
I wrote a script to check the OrderCalcMargin() function. The function returns zero for some symbols.

This is probably for those symbols that are not in MarketWatch, as SymbolName is said to be for SymbolName:

SymbolName

Returns the name of the specified symbol.

stringSymbolName(
intpos,// number in the list
bool selected// true - only symbols in MarketWatch
);

Parameters

pos

[in] Symbol number in order.

selected

[in] Query mode. If true, then the symbol is taken from the list of selected in MarketWatch. If the value is false, then the symbol is taken from the common list.

Returned value

Value of string type with symbol name.

Print the name of the symbol for which you get an unexpected result and compare it to the list in MarketWatch.
Reason: