Jump to content

Recommended Posts

Hello 🙂

 

At the beginning: thanks to all of you for many very informative discussions on this forum. When working in D7 in most cases Ctrl+F1 was the right answer to most problems. But now I'm more and more frequently saved by the info provided by members of Delphi-PRAXiS 🙂

 

And now my actual pain: I'm using Delphi 11 CE ver 28.0. While working with quite simple code I have encountered two strange, fully reproducible problems. The description and related code are below. Additionally all the details and both problems are presented within a short video: https://drive.google.com/file/d/147uSLerfnu-3DZME1jHQ8nsnlAdqtNRi (the code is compiled without optimization). Maybe someone wise can shed some light on the sources of those problems and how to get rid of them (excluding "Embarcadero" problem) ?

 

All the best!

 

---------
>>> The example task:

 

The square matrix TConfusionMatrix = array[0..x,0..x] of Extended is created in two ways (please see the code below): 
- with primitive functions such as Cmtx33() or Cmtx22() which place the right values in the right cells of the matrix, or
- by splitting string representation of the matrix (with function MtxStr2ConfusionMatrix). Example string representation of the 3x3 matrix is: '[0,1,2, 3,4,5, 6,7,8]' (rows concatenation, spaces and brackets do not matter).

 

Created matrices should represent structure "array of rows" and oboth methods should fill the matrix in the same way. For example, if cmtx and cmtx2 are of TConfusionMatrix type then after calls: MtxStr2ConfusionMatrix('[43,0,1,0,45,0,2,0,44]', cmtx, class_cnt); cmtx2:=Cmtx33(43,0,1,0,45,0,2,0,44); variables cmtx and cmtx2 should include the same values in the same elements of matrices (among those which were modified by both functions). Additionally class_cnt will include the size of the row/column of the matrix (in this example class_cnt will be equal 3). It is assumed that strings representing the matrix will always have correct structure and that will include the right number of elements to create a square matrix (so we do not need to think about such things here).

 

>>> Encountered problems:

 

1.) The first problem ("the inspection problem"):
Depending on the order of calls to MtxStr2ConfusionMatrix() and Cmtx33() the results of the former function differ.
Sometimes after the call of MtxStr2ConfusionMatrix() the created matrix is displayed in inspection window as it would be (improperly) transposed.
But after next call of Cmtx33() the matrix set by MtxStr2ConfusionMatrix() starts to look properly. 
Further comparison of matrices created with both functions indicate that they are equal and related code execution reacts accordingly.

 

2.) The second problem ("the wtf problem"):
But sometimes - for exactly the same data - the matrix being the result of MtxStr2ConfusionMatrix() permanently stays improperly transposed and the matrices created with both methods are recognised as different. Then this breaks the logic of the code. I'm blind or something but I can't see any obvious reason for such behaviour.
Results from primitive functions Cmtx33(), Cmtx22() are always correct and presented properly.

 

----------
>>> The code:

const MAX_CLASSES = 50;
type TConfusionMatrix = array[0..MAX_CLASSES,0..MAX_CLASSES] of Extended;



//transforms square matrix in the form of string to a table of rows;
//cmtx_str = '[0,1,2,3,4,5,6,7,8]' => [[0,1,2][3,4,5][6,7,8]]
//(row and column of index=0 are left for other purposes)
function MtxStr2ConfusionMatrix(cmtx_str :String; var cmtx :TConfusionMatrix; var class_cnt :Word) :Boolean;
var i,j :Word;
    splittedString : TStringDynArray;
    res :Boolean;
begin
  res:=True;
  try
    RemoveChar(cmtx_str,' ');
    RemoveChar(cmtx_str,'[');
    RemoveChar(cmtx_str,']');
    class_cnt:=0;

    splittedString:=SplitString(cmtx_str, ',');
    if Length(splittedString)<4 then res:=False
    else
    begin
      if Frac(Sqrt(Length(splittedString)))<0.01 then
        class_cnt:=Round(Sqrt(Length(splittedString)))
      else res:=False;
    end;

    if class_cnt>=2 then
    begin
      EraseConfusionMatrix(cmtx,class_cnt,MAX_CLASSES,0);
      for i:=1 to class_cnt do
        for j:=1 to class_cnt do
          cmtx[i,j]:= StrToFloat(splittedString[i+(j-1)*class_cnt - 1]);
    end;
  except
    res:=False;
  end;
  result:=res;
end;



function CompareConfusionMatrices(c1,c2 :TConfusionMatrix; class_cnt :Word) :Boolean;
var i,j :Integer;
    ok :Boolean;
begin
  ok:=True;
  for i:=1 to class_cnt do
    for j:=1 to class_cnt do
      if c1[i,j]<>c2[i,j] then
      begin
        ok:=False;
        break;
      end;
  result:=ok;
end;



function Cmtx33(c11,c12,c13,c21,c22,c23,c31,c32,c33 :LongInt) :TConfusionMatrix;
begin
  cmtx[1,1]:=c11;
  cmtx[1,2]:=c12;
  cmtx[1,3]:=c13;
  cmtx[2,1]:=c21;
  cmtx[2,2]:=c22;
  cmtx[2,3]:=c23;
  cmtx[3,1]:=c31;
  cmtx[3,2]:=c32;
  cmtx[3,3]:=c33;
  result:=cmtx;
end;



function Cmtx22(c11,c12,c21,c22 :LongInt) :TConfusionMatrix;
begin
  cmtx[1,1]:=c11;
  cmtx[1,2]:=c12;
  cmtx[2,1]:=c21;
  cmtx[2,2]:=c22;
  result:=cmtx;
end;



procedure RemoveChar(var s :String; ch :Char);
var d :String;
    i,ls :LongInt;
begin
  d:='';
  ls:=Length(s);
  i:=1;
  for i:=1 to ls do
    if s[i]<>ch then d:=d+s[i];
  s:=d;
end;



procedure EraseConfusionMatrix(var cmtx :TConfusionMatrix; num_classes, max_classes :Word; value :Integer);
var i,j :Word;
begin 
  if num_classes<=max_classes then
  begin
    for i:=0 to num_classes do
      for j:=0 to num_classes do cmtx[i,j]:=value;
  end
  else ShowMessage('Confusion Matrix can not be greater than '+IntToStr(max_classes)+'x'+IntToStr(max_classes));
end;

 

===EOT===
 

Share this post


Link to post

One thing may be a problem. Both the cmtx22 and cmtx33 functions use a (global ?) variable cmtx but do not clear it first. So the array elements not set by the functions will have values carried over from previous uses of this variable.

 

Sorry, but the whole design looks fishy and error prone to me and seems to use plain procedural programming 40 years out of date. You define a large fixed size array type, variables of which will not only waste a lot of memory for elements never used, but you also have no convenient way to figure out from a variable what the dimensions of the actually used part are. In my opinion you should build a TConfusionMatrix class, which internally uses a 2D dynamic array (array of array of extended) to store the values of the matrix. The constructor of the class would take the needed dimension and size the internal array accordingly. You could have additional constructors to build the matrix from your string representation or a 1D array of values, methods to return a string representation, to compare an object of the class to another, properties to return the dimensions of the matrix or to get or set element values. Since the matrix object knows the size of the internal array it would be easy to check report indexing errors and so on.

  • Like 2

Share this post


Link to post

Dear Peter Below. Thank you for pointing out the usage of global cmtx variable in cmtx22/cmtx33 functions. I need to check, but most probably this is the cause of at least part of the observed problem. I was blinded by the spaghetti 🙂

Btw. The way of data representation and the fact that the code is almost fully procedural (no OOP) is here by purpose - to achieve very high speed of computation - and the result is more than good. This includes also usage of global variables in some cases. The price: much higher cost of maintenance and the related pain. 

 

 

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×