StackOverflow2013

Note that there are some explanatory texts on larger screens.

plurals

PO
text
Body
copied!<p>Some general suggestions:</p> <ul> <li>Name variables in <strong>English</strong>. Give a short explanation if it is not immediately clear, what they are indented for. What is <code>jum_bag</code> for example? For me <code>uk_pop</code> is music style.</li> <li>Write comments in <strong>English</strong>, even if you develop source code only for yourself. If you ever have to share your code with a foreigner, you will spend a lot of time explaining or re-translating. I would like to know for example, what <code>%batasan tidak boleh</code> means. Probably, you describe here that this is only a quick hack but that someone should really check this again, before going into production.</li> </ul> <p>Specific to your code:</p> <ul> <li>Its really easy to confuse <code>gab1</code> with <code>gabh1</code> or <code>gabb1</code>.</li> <li>For me, <code>krom</code> is too similar to the built-in function <code>kron</code>. In fact, I first thought that you are computing lots of tensor products.</li> <li><p><code>gab1 .. gab4</code> are probably best combined into an array or into a cell, e.g. you could use</p> <pre><code>gab = cell(1, 4); for ii = ... gab{1}(:,:,ii) = sum(krom(:,:,ii)==1); gab{2}(:,:,ii) = sum(krom(:,:,ii)==2); gab{3}(:,:,ii) = sum(krom(:,:,ii)==3); gab{4}(:,:,ii) = sum(krom(:,:,ii)==4); end </code></pre> <p>The advantage is that you can re-write the comparsisons with another loop. It also helps when computing <code>gabh1</code>, <code>gabb1</code> and <code>tot</code> later on. </p> <p>If you further introduce a variable like <code>highestNumberToCompare</code>, you only have to make one change, when you certainly find out that its important to check, if the elements are equal to 5 and 6, too.</p></li> <li><p>Add a semicolon at the end of every command. Having too much output is annoying and also slow.</p></li> <li><p>The <code>numel(find(gabb1 ~= 2 & gabb1 ~= 0))</code> is better expressed as <code>sum(gabb1(:) ~= 2 & gabb1(:) ~= 0)</code>. A <code>find</code> is not needed because you do not care about the indices but only about the number of indices, which is equal to the number of <code>true</code>'s.</p></li> <li><p>And of course: This code</p> <pre><code>for ii=1:uk_pop gab1(:,:,ii) = sum(krom(:,:,ii)==1) end </code></pre> <p>is <strong>really, really</strong> slow. In every iteration, you increase the size of the <code>gab1</code> array, which means that you have to i) allocate more memory, ii) copy the old matrix and iii) write the new row. This is <strong>much</strong> faster, if you set the size of the <code>gab1</code> array in front of the loop:</p> <pre><code>gab1 = zeros(... final size ...); for ii=1:uk_pop gab1(:,:,ii) = sum(krom(:,:,ii)==1) end </code></pre> <p>Probably, you should also re-think the size and shape of <code>gab1</code>. I don't think, you need a 3D array here, because <code>sum()</code> already reduces one dimension (if <code>krom</code> is 3D the output of <code>sum()</code> is at most 2D).</p> <p>Probably, you can skip the loop at all and use a simple <code>sum(krom==1, 3)</code> instead. However, in every case you should be really aware of the size and shape of your results.</p></li> </ul> <p><strong>Edit inspired by <a href="https://stackoverflow.com/users/1085062/rody-oldenhuis">Rody Oldenhuis</a>:</strong></p> <p>As Rody pointed out, the 'problem' with your code is that its highly unlikely (though not impossible) that you create a matrix which fulfills your constraints by assigning the numbers randomly. The code below creates a matrix <code>temp</code> with the following characteristics:</p> <ul> <li>The numbers <code>1 .. maxNumber</code> appear either twice per column or not at all.</li> <li>All rows are a random permutation of the numbers <code>1 .. B</code>, where <code>B</code> is equal to the length of a row (i.e. the number of columns).</li> </ul> <p>Finally, the <code>temp</code> matrix is used to fill a 3D array called <code>result</code>. I hope, you can adapt it to your needs.</p> <pre><code>clear all; A = 8; B = 12; C = 10; % The numbers [1 .. maxNumber] have to appear exactly twice in a % column or not at all. maxNumber = 4; result = zeros(A, B, C); for ii = 1 : C temp = zeros(A, B); for number = 1 : maxNumber forbiddenRows = zeros(1, A); forbiddenColumns = zeros(1, A/2); for count = 1 : A/2 illegalIndices = true; while illegalIndices illegalIndices = false; % Draw a column which has not been used for this number. randomColumn = randi(B); while any(ismember(forbiddenColumns, randomColumn)) randomColumn = randi(B); end % Draw two rows which have not been used for this number. randomRows = randi(A, 1, 2); while randomRows(1) == randomRows(2) ... || any(ismember(forbiddenRows, randomRows)) randomRows = randi(A, 1, 2); end % Make sure not to overwrite previous non-zeros. if any(temp(randomRows, randomColumn)) illegalIndices = true; continue; end end % Mark the rows and column as forbidden for this number. forbiddenColumns(count) = randomColumn; forbiddenRows((count - 1) * 2 + (1:2)) = randomRows; temp(randomRows, randomColumn) = number; end end % Now every row contains the numbers [1 .. maxNumber] by % construction. Fill the zeros with a permutation of the % interval [maxNumber + 1 .. B]. for count = 1 : A mask = temp(count, :) == 0; temp(count, mask) = maxNumber + randperm(B - maxNumber); end % Store this page. result(:,:,ii) = temp; end </code></pre>

Querying!

Guidance

An individual column

Larger individual text columns get their own page to allow for proper reading.

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload