How to make the code more efficient: Group experimental result into 3 groups

Dear Coder, This is not a critical problem, however I am interested to know if there is a possibility to make this code more compact and efficient. I learn to code better by getting feedback from the community. This is a made up experimental protocol. During the experiment, 15 subjects were randomize into 3 conditions (i.e., COND1A, COND2A, COND1B). That is 5 subjects per condition. The data collection were conducted separately for 12 days (i.e., SD_BL, SD_1, SD_2, ... SD_11). The main objective of this program is to classify the subject state at a particular time into excellent, good or poor (label as 1, 2, 3, respectively) depending on the task performance (TASK_ONE, TASK_TWO or TASK_Three). For simplicity, we only consider TASK_TWO, Subject: S1A, COND: COND1A, and data collection at SD_BL. Then, the result of TASK_TWO are [ 199.4 203.7 184.89 202.5 217.2 196.1]

To group into the 3 categories, the threshold between the group was obtained by the difference between minimum and maximum value of the TASK_TWO performance was divided by 3 (e.g., 199.4 -184.89/3 = 4.83). Thus, the performance is consider poor if its value between 184.89 to 189.7 (184.89+4.83 = 189.7) and excellent if the value of TASK_TWO fall between 194.57 to 199.4S. With this in mind, the following code were realize

function [at] = SplitGroup (at)

for i=1:length(at.Valindex)
    indexLoc =at.Valindex(:,i).';
    BB = length(indexLoc {1,1});
    m =zeros (1, BB);
    for j=1:BB
    m(j)=at.PatData{(indexLoc{1,1}(j,1)),at.AccessColumn };  end
    [c1,~]=min(m);
    [c2,~]=max(m);

      shiftVal = (abs(c1-c2))/at.interval;
      at.C1Thrshld = abs (shiftVal + c1);
      at.C2Thrshld = abs (shiftVal - c2);
      S =zeros (1, BB);
    % C3 any value between C1Thrshld & C2Thrshld  | c1--(Class1)--C1Thrshld--(Class2)--C2Thrshld--(Class3)--c2

     S(m<=at.C1Thrshld) = 1;
     S(m>=at.C1Thrshld & m <=at.C2Thrshld) = 2;
     S(m>at.C2Thrshld ) = 3;
     at.temp (((indexLoc{1,1}).'))=S(1:length(S));

end
end

I really appreciate any idea how to make this code more efficient and compact. The complete code and MAT file was attached together in this thread

Answer 1

[Jan]

Some ideas for nicer code:

    indexLoc =at.Valindex(:,i).';
    BB = length(indexLoc {1,1});

You access indexLoc{1,1} only, so why do you extract it as cell vector at all? What about:

    indexLoc = at.Valindex{i};

and using "indexLoc" instead of "indexLoc{1,1}" afterwards? But what is the purpose of the transposition then?

The linear indexing is slightly faster than using the 2 indices for vectors:

m(j)=at.PatData{(indexLoc{1,1}(j,1)),at.AccessColumn };
% Cleaner:
m(j) = at.PatData{indexLoc(j)), at.AccessColumn};  % With the above change

Replace

[c1,~]=min(m);

by

c1 = min(m)

In

     S(m<=at.C1Thrshld) = 1;
     S(m>=at.C1Thrshld & m <=at.C2Thrshld) = 2;

the element m==at.C1Thrshld belongs to two intervals.

     S = zeros(1, BB);
     S(m <= at.C1Thrshld)                    = 1;
     S(m > at.C1Thrshld & m <= at.C2Thrshld) = 2;
     S(m > at.C2Thrshld)                     = 3;
     at.temp(indexLoc) = S;

S and S(1:length(S)) is the same for vectors. For matrices the later might be confusing.

Another option, which could be faster, but slower also:

     S                    = repmat(3, 1, BB);
     S(m <= at.C2Thrshld) = 2;
     S(m <= at.C1Thrshld) = 1;
     at.temp(indexLoc)    = S;

Compare this with a tic/toc:

tic
for kk = 1:1e5
  ...
end
toc

or with timeit.

I prefer a standard indentation scheme and spaces around operators, but not between commands and variables and the corresponding parenthesis. If you prefer another scheme, this is okay, but following it strictly will increase the readability of the code remarkably.

[balandong]

HI Jan, Thanks for the time in entertaining this thread. Really appreciate it. I am interested, particulary the following code

       S                    = repmat(3, 1, BB);
       S(m <= at.C2Thrshld) = 2;
       S(m <= at.C1Thrshld) = 1;
       at.temp(indexLoc)    = S;

By the way, I am having hard time understanding what you try to convey about scheme ' prefer a standard indentation scheme and spaces around operators, but not between commands and variables and the corresponding parenthesis'. What do you mean by standard indentation scheme?.

On separate issue, do you think the lines under FUNCTION INDEXINGVALUE & INDLOC can be further improvise?

Any how, thanks for the response.

[Jan]

"Standard indentation scheme": Matlab can indent the code automatically, such that e.g. FOR, IF and WHILE blocks can be identified on the first view. Hit Ctrl-a, Ctrl-i in the editor to see the effect. It improves the readability. In Matlab's preferences the automatic indentation during editing can be enabled.

I do not understand what "the lines under FUNCTION INDEXINGVALUE & INDLOC" are.

Without having real input data, an optimizing of run time is based on guessing only. The actual sizes and values matter. I'm still confused about the type and size of at.Valindex.

Summary:

function at = SplitGroup(at)
% Add meaningful comments!

for i = 1:size(at.Valindex, 2)   % Safer than LENGTH
  index = at.Valindex{k};     % Perhaps
  m  = cat(2, at.PatData{index, at.AccessColumn});
  c1 = min(m);
  c2 = max(m);
  D  = (c2 - c1) / at.interval;
  %
  S                   = repmat(3, 1, numel(m));
  S(m <= abs(D - c2)) = 2;
  S(m <= abs(D + c1)) = 1;
  at.temp(index)      = S;
end
end

Perhaps pre-allocating at.temp is essential, but it depends on the values of indexLoc and if the field is defined before.