Rank: 3935 based on 6 downloads (last 30 days) and 1 file submitted
photo

Guy

E-mail

Personal Profile:

Professional Interests:

 

Watch this Author's files

 

Files Posted by Guy
Updated   File Tags Downloads
(last 30 days)
Comments Rating
27 Aug 2009 mexSparseLogical0Diag Change all the elements on the main diagonal of a logical sparse matrix to 0. Author: Guy logical, mex, sparse 6 2
Comments and Ratings by Guy
Updated File Comments Rating
28 Aug 2009 mexSparseLogical0Diag Change all the elements on the main diagonal of a logical sparse matrix to 0. Author: Guy

Thanks for the tips James!

I especially appreciate the tips regarding where failure could occur (using bool instead of mxLogical, and the nlhs test).

The rest of the comments are also helpful (even though I doubt that this code will be used out of its context very often, so I won't bother to change things like memory allocation).

When I get back to university in a week or so, I'll definitely fix the error causing issues.

Thanks!

Comments and Ratings on Guy's Files View all
Updated File Comment by Comments Rating
28 Aug 2009 mexSparseLogical0Diag Change all the elements on the main diagonal of a logical sparse matrix to 0. Author: Guy Guy

Thanks for the tips James!

I especially appreciate the tips regarding where failure could occur (using bool instead of mxLogical, and the nlhs test).

The rest of the comments are also helpful (even though I doubt that this code will be used out of its context very often, so I won't bother to change things like memory allocation).

When I get back to university in a week or so, I'll definitely fix the error causing issues.

Thanks!

28 Aug 2009 mexSparseLogical0Diag Change all the elements on the main diagonal of a logical sparse matrix to 0. Author: Guy Tursa, James

A few suggestions for improvement:

1) Don't use bool to point to the MATLAB logical data. MATLAB supplies a data type for this, mxLogical. Using bool only sets you up for an unexpected failure on machines where sizeof(bool) is not 1.

2) Don't use nz to allocate the memory for the new array. There might be a lot of wasted space involved in the input array and there is not point in propagating that waste. Instead, just go get the actual number of current non-zero elements and use that, since you know your result will not be larger than this. e.g.,

    mwSize n, n2;
    jc = mxGetJc(prhs[0]);
    n = mxGetN(prhs[0]);
    nz = jc[n];
        :
    v = mxCreateSparseLogicalMatrix (m, n, nz);

3) There is no code to realloc the memory if a large percentage was reset to zero. e.g., an input matrix that is nearly the same as an identity matrix. Although not an error, you might think about including this.

4) I know you supplied an m-file with the help (very good), but it might be good to copy the calling info into the cpp source file so that a user can have this all in one file when editing the source. Again, not an error but it helps others to read your code.

5) Your nlhs test isn't the best, as it doesn't cover some possible erroneous inputs (e.g., what if user tries to fill two output variables?):

if (nlhs > 0)
plhs[0] = v;

 A better method is as follows:

// at the beginning of your code:
    if( nlhs > 1 ) {
        mexErrMsgTxt("Too many outputs, expecting at most one.");
}

// at the end of your code
    plhs[0] = v;

i.e., at the end of your code don't do any test, just set plhs[0]. That way it will work if the user doesn't try to set the result to any variable ... in that case the result simply goes into ans. This always works, even if nlhs == 0.

6) You might consider making this a self-building mex routine. For an example of this, see one of my submissions, e.g.,

http://www.mathworks.com/matlabcentral/fileexchange/22239

7) With some very minor tweaking, you could also supply a C source code version so the users that only have access to the supplied lcc compiler can use this out of the box.

The code appears to work fine, and is indeed much faster than a loop. I would probably rate it a 3 at this point, but I will wait for author's response to my comments before rating and may go higher.

Top Tags Applied by Guy
logical, mex, sparse
Files Tagged by Guy
Updated   File Tags Downloads
(last 30 days)
Comments Rating
27 Aug 2009 mexSparseLogical0Diag Change all the elements on the main diagonal of a logical sparse matrix to 0. Author: Guy logical, mex, sparse 6 2
 

MATLAB Central Terms of Use

NOTICE: Any content you submit to MATLAB Central, including personal information, is not subject to the protections which may be afforded information collected under other sections of The MathWorks, Inc. Web site. You are entirely responsible for all content that you upload, post, e-mail, transmit or otherwise make available via MATLAB Central. The MathWorks does not control the content posted by visitors to MATLAB Central and, does not guarantee the accuracy, integrity, or quality of such content. Under no circumstances will The MathWorks be liable in any way for any content not authored by The MathWorks, or any loss or damage of any kind incurred as a result of the use of any content posted, e-mailed, transmitted or otherwise made available via MATLAB Central. Read the complete Terms prior to use.

Contact us at files@mathworks.com