Slow Triple for loop

1 view (last 30 days)
James
James on 17 Jul 2013
Hi,
I have created a program that has a tripple for loop but runs very slowly and was hoping for some tips to speed it up... Here is the code:
if WeightingVectorWells(1) > 0 || WeightingVectorWells(4) > 0
for LatCounter = 1:length(handles.Latitude)
for LongCounter = 1:length(handles.Longitude)
for WellDataCounter = 1:size(handles.WellData,1)
if sqrt(((handles.WellData{WellDataCounter,2} - handles.Latitude(LatCounter)) ^ 2 + (handles.WellData{WellDataCounter,3} - handles.Longitude(LongCounter)) ^ 2)) < .01067
handles.ScoreSheetBPDOil(LatCounter,LongCounter) = handles.WellData{WellDataCounter,12} * WeightingVectorWells(1) + ...
handles.ScoreSheetBPDOil(LatCounter,LongCounter);
handles.ScoreSheetCumOil(LatCounter,LongCounter) = handles.WellData{WellDataCounter,13} * WeightingVectorWells(4) + ...
handles.ScoreSheetCumOil(LatCounter,LongCounter);
end
end
end
if LatCounter == length(handles.Latitude)
LatCounter = LatCounter - 1;
end
progressbar(LatCounter / length(handles.Latitude))
end
end
I created a GUI for this program (first time using GUIDE and handles and handles was the easiest way I found to talk to different functions and share data so sorry if that's bad pratice). The handles.ScoreSheets are preallocated. The LatCounter and LongCounter are both about 300 in size and the handles.WellData is a 19000 by 26 matrix (19000 being the max, it can be as little as 1 but is usually around 500 or so). Whenever I stop this mid processing (ctrl-c) it always seems to be doing the sqrt function. Also, as a side note I'm trying to get my company to get parallel processing tool box because I have six of these loops. progressbar is just a function I picked up off the file exchange and takes very little processing and the if statement at the bottom inside the outer most loop is just so that progressbar doesn't bug out.
Thanks for any help, I'm an engineer, not a programmer so I really do appreciate anything.

Accepted Answer

Cedric
Cedric on 17 Jul 2013
Edited: Cedric on 17 Jul 2013
The very first thing is that you should not modify the loop index LatCounter within the loop. ( EDIT: the following sentence is wrong! see Iain's comment) What you are doing with the -1 will create an infinite loop blocked on LatCounter == length(handles.Latitude).
Then you can probably get rid of the two outer loops by creating a meshgrid of longitudes and latitudes. Example:
lat = 1:2:7
lon = 10:2:14
[LAT, LON] = meshgrid(lat, lon)
normCoordSq = LAT.^2 + LON.^2
cumResult = zeros(size(LAT)) ;
data = [2, 7, 5, 9] ;
for dataId = 1 : length(data)
mask = sqrt(normCoordSq + cumResult + data(dataId)^2) < 16 ;
cumResult(mask) = cumResult(mask) + data(dataId)^2 ;
end
cumResult
This leads to the following output:
lat =
1 3 5 7
lon =
10 12 14
LAT =
1 3 5 7
1 3 5 7
1 3 5 7
LON =
10 10 10 10
12 12 12 12
14 14 14 14
normCoordSq =
101 109 125 149
145 153 169 193
197 205 221 245
cumResult =
78 78 78 78
78 78 78 53
53 29 29 4
As you can see, there is only one loop, and the conditional statement is replaced by logical indexing based on the outcome of a relational operation. Then you could probably get rid of the last loop using ARRAYFUN, CELLFUN, or BSXFUN, but I doubt that it would bring much improvement (in the sense that these are essentially hidden loops which bring more conciseness than efficiency in term of computation time).
  11 Comments
Cedric
Cedric on 18 Jul 2013
Edited: Cedric on 18 Jul 2013
I am not programming GUI in MATLAB actually (just tried once), but based on what I am doing in other languages, I would look for a way to work with IDs instead of strings, and keep strings either for display or for when users are allowed to type county names (which have to be compared using e.g. STRCMPI to the list of all counties for validation). Here are a few remarks..
If multiple entries/rows in the Excel file can be associated with similar counties, I would use UNIQUE to get a list of unique county names. UNIQUE outputs not only the list, but also arrays of indices that allow you to go back and forth between the original list and the list of unique names. Note that UNIQUE sorts too by default.
SORT could also be used, and it also outputs an array of indices.
In the following, I assume that you have a cell array of unique, sorted county names, named countyName. Let's also assume that you have four counties, named 'A', 'B', 'C', and 'D'. One way to work with IDs is as follows: you create arrays of IDs and you work with them, leaving countyName unchanged. To illustrate
county_displayID = 1 : length(countyName) ; % Display in left listbox.
county_deleteID = [] ; % Display in right listbox.
Now for building the left listbox:
if ~isempty(county_displayID)
set(handles.listboxLeft, 'string', countyName(county_displayID)) ;
end
For building the right listbox:
if ~isempty(county_deleteID)
set(handles.listboxRight, 'string', countyName(county_deleteID)) ;
end
For moving a county from the left to the right listbox:
% Get left listbox index and corresponding county ID.
listboxID_left = get(handles.listboxLeft, 'value')
countyID = county_displayID(listboxID_left) ;
% Remove this element from displayIDs.
county_displayID(listboxID_left) = [] ;
% Add it to deleteIDs.
county_deleteID = [county_deleteID, countyID] ;
In your context, if WellData (I let you add the "handles.") is what I call countyName but with extra columns, you can delete chosen rows just by doing:
WellData(county_deleteID,:) = [] ;
If WellData is unsorted or not unique, you have just one additional step to do using arrays of indices outputted by UNIQUE or SORT to translate county_deleteID back to the original indexing scheme.
Hope it helps,
Cedric
James
James on 19 Jul 2013
Thanks for the help Cedric. Looks like a good solution to me.

Sign in to comment.

More Answers (1)

Iain
Iain on 17 Jul 2013
1. Do the calculations as few times as possible: The following should be in the outer loop. - Unless you want latcounter to be allowed to be the maximum value for a single calculation.
if LatCounter == length(handles.Latitude)
LatCounter = LatCounter - 1;
end
2. Minimize the amount of calcs:
sqrt(x) < y, could be x < y^2 - given that y^2 is a constant, you've removed the sqrt calc from the loop.
3. Vectorise wherever possible.
Your inner loop can be REMOVED and replaced with.
index = ((([handles.WellData{:,2}] - handles.Latitude(LatCounter)) ^ 2 + ([handles.WellData{:,3}] - handles.Longitude(LongCounter)) ^ 2)) < .01067^2;
handles.ScoreSheetBPDOil(LatCounter,LongCounter) = sum([handles.WellData{index,12}]) * WeightingVectorWells(1);
handles.ScoreSheetCumOil(LatCounter,LongCounter) = sum( [handles.WellData{index,13}]) * WeightingVectorWells(4);
3a. Its easier to vectorise if your variables are straightforward numeric arrays (handles.WellData is not) 3b. With thought, you can use elementwise operations and bsxfun, to remove all three loops.

Community Treasure Hunt

Find the treasures in MATLAB Central and discover how the community can help you!

Start Hunting!