current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite

I have a very big Matlab simulation project in my hands, which I wanted to optimize, since I'm running it many times to tune parameters and the like.

Using Matlab's profile I identified one function that is eating up most of my time, specifically the output(i,1) assignment line.

This function is called a LOT, where input is a 10x1 double passed as an argument, and output is also a 10x1 vector.

function output = my_function(input)

a = size(input,1);
output = input*0;
dens = density(input);

% for each i, output(i) is the maximum between output(i+1) and mean(output(i+1:end))
for i = 1:a-1
    output(i,1)= max(mean(dens(i+1:a,1)),dens(i+1,1));
end
output(a,1) = dens(a,1);

end

density() function:

function dens = density(input)

dens = 1000*(1 - (input+288.9414)./(508929.2.*(input+68.12963)).*(input-3.9863).^2);

end

My ideas:

  • I think vectorization would maybe help to get rid of the loop, but I'm not familiar at all with the technique.
  • Is there a faster/alternative way to calculate the mean (maybe without Matlab's built-in function call?)
share|improve this question
    
I'm assuming you have your own density function defined? It isn't defined in the standard MatLab functions –  syb0rg Dec 5 '14 at 18:15
    
Yes, density is an external function I declared. It is not relevant for my question, as dens is also a 10x1 double vector –  titus.andronicus Dec 5 '14 at 18:16
    
The line that eats up 90% of the resources of the script is output(i,1)= max(mean(dens(i+1:a,1)),dens(i+1,1));, thus the vectorization I mentioned –  titus.andronicus Dec 5 '14 at 18:31
1  
What density does matters because you might be able to calculate output(i,1) from values in output rather than taking a mean of values in dens. My offhand thought is that you are iterating in the wrong direction. Given what you are doing, you should iterate down not up and calculate the mean as you go. Someone like syb0rg with more knowledge of Matlab could probably give you better help if you offered a working example with all code. –  Brythan Dec 5 '14 at 21:17
    
@Brythan, @sybOrg I'm so sorry, I really thought it wasn't necessary. I added the density function in the last edit –  titus.andronicus Dec 5 '14 at 22:14

2 Answers 2

up vote 3 down vote accepted

A few notes:

  • I'm confused about the design of your function. If you are only processing column vectors, why not just process a row vector instead? That's what I would expect when calling this function.

     my_function([1:2:20]') % how I have to call your function; convoluted and unexpected

 my_function(1:2:20) % how I expect to call your function; cleaner

  • Vectorization is definitely the way to go for efficiency in MatLab. One way to vectorize code is to use the : operator, which it seems you know how to use.

    Unfortunately, we can not nest colon operators in Matlab, so I do not know of a way to vectorize your code. Break apart how the colon operator works and how your code works to see why

    % (i + 1):a where i is 1:(a - 1) [a is the length, 10 in this case]
2 3 4 5 6 7 8 9 10 % 2:10
3 4 5 6 7 8 9 10 % 3:10
4 5 6 7 8 9 10 % 4:10
5 6 7 8 9 10 % 5:10
% and so on

% (2:10):10
2 3 4 5 6 7 8 9 10 % doesn't quite work... have to use a loop

    There may be some other way to accomplish what you are trying to do, but I can't think of a different and more efficient way to approach it

  • You should be putting brackets around the return value(s) in your function declaration. They can be omitted, but it's better practice to keep them there.

  • Your current way of getting the size of your vector isn't how I would go about it.

    a = size(input,1);

    You are always going to have one of the dimensions be of size 1, correct? So why not use the length() function instead, which always returns the greatest of the number of dimensions passed to it.

    a = length(input);

  • You have a clever way of getting a zeroed out vector of the same length.

    output = input*0;

    I'm not sure how the performance would compare, but using the function zeros() may be faster.

  • You don't need to specify that 1 when indexing a vector, MatLab knows.

    a = size(input,1);
a = size(input); % almost the same, is the same for your purposes

  • Don't use MatLab's profiler to measure the performance of your code. Why? Because in measuring the code, it also happens to measure itself running the profiling tests and compiling the results, which can take up quite some time on some machines (including mine).

    The more accepted way to measure how long code takes to run is to use tic and toc, as such:

    >> tic; my_function([1:2:20]'); toc;
Elapsed time is 0.000976 seconds.

    Make sure you put them on the same line though like I did above, or it will also count the time it takes you to type out the function and execute it, as well as typing out toc and executing that.

    >> tic;
>> my_function([1:2:20]');
>> toc;
Elapsed time is 13.184153 seconds.
share|improve this answer
    
thanks for your suggestions, will give them a try :) –  titus.andronicus Dec 6 '14 at 8:14
    
I've edited the question with a solution proposal. Not sure why the residual happens... –  titus.andronicus Dec 8 '14 at 11:48
up vote 2 down vote

Warning: I don't have Matlab installed and haven't done much with it, so my syntax may be off. 

% for each i, output(i) is the maximum between output(i+1) and mean(output(i+1:end))
for i = 1:a-1
    output(i,1)= max(mean(dens(i+1:a,1)),dens(i+1,1));
end
output(a,1) = dens(a,1);

If I understand that correctly, the % line is a comment. Then you have a for loop from 1 to a-1 where you assign all but the last entry of output. After the loop finishes, you assign the last entry of output.

The downside of this approach is that you have to sum up and divide the array for every entry. I think it would be better to do the end first and keep a running sum. I'm not sure of syntax, but I would expect that to look something like 

count = 1;
sum = dens(a,1);
output(a,1) = dens(a,1);

for i = a-1:-1:2
    output(i,1) = max((sum / count), dens(i+1,1));
    sum = sum + dens(i,1);
    count = count + 1;
end

output(1,1) = max(sum / count), dens(2,1));

Again, apologies if my syntax doesn't work. Hopefully this illustrates what I'm trying to do.

I stuck with the syntax that you were using in case I didn't understand syb0rg's comments properly.

Note: if you do this, don't forget to re-profile. It may be that there is an optimization on calculating the mean that outstrips the theoretical advantage of this approach.

share|improve this answer
    
The main point that bugged me with the question was that the mean function is called all the time, and it could probably be computed beforehand, to make the main loop faster. Your approach is similar to what I had in mind. –  Gürkan Çetin Jul 15 at 20:34

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.