C Traps and Pitfalls

Here’s a concise paper describing common C programming pitfalls by Andrew Koening (www.literateprogramming.com/ctraps.pdf) corresponding to be book with the same title.

As a reminder to myself, I’ll spend this page summarizing common mistakes and my history with it.

Here are the mistakes that I don’t make because of certain programming habits:

  • Operator precedence: I use enough parenthesis to not rely on operator precedence
  • Pointer dereferencing: I always do *(p++) instead of *p++ unless it’s idiomatic.
  • for() or if() statements executing only first line: I always surround the block with {} even if it’s just one line. Too often we need to inject an extra line and without {} it becomes a trap.
  • Undefined side effect order: I never do something like y[i++]=x[i]
  • char* p, q: very tempting since C++ style emphasize on pointer as a type over whether the variable is a pointer. I almost never declare multiple variables in one line.
  • Macro repeating side effects: use inline functions instead whenever possible. Use templates in C++.
  • Unexpected macro associations: guard expressions with (). Use typedef.

Did these once before, adjusted my programming habits to avoid it:

  • counting down in for() loop with unsigned running variable: I stick with signed running variables in general. If I’m forced to use unsigned, I’ll remind myself that I can only stop AFTER hitting 1, but not 0 (i.e. i=0 never got executed).

Haven’t got a chance to run into these, but I’ll program defensively:

  • Integer overflow: do a<b instead of (a-b)<0. Calculate mean by adding halfway length to the smaller number (i.e. (a+b)/2 == a + (b-a)/2 given a<b). Shows up in binary search.
  • Number of bits to shift is always unsigned (i.e. -1 is a big number!)

What I learned from the paper:

  • stdio buffer on stack (registered with setbuf()) freed before I/O flushed: use static buffer (or just make sure the buffer lives outside the function call).
  • char type might be signed (128 to 255 are -128 to -1) so it sign extends during upcast. Use unsigned char go guarantee zero extend for upcasting.
  • toupper()/tolower() might be implemented as a simple macro (no checks, incorrect /w side effects)
  • Can index into a string literal: "abcdefg"[3] gives 'd'

Mistakes that I usually make when I switch back from full-time MATLAB programming:

  • Logical negation using ~ operator instead of ! operator.

Common mistakes I rarely make because of certain understanding:

  • Forgetting to break at every case in switch block. It’s hard to forget once you’re aware of the Duff’s device.
  • sizeof(a[])/sizeof(a[0]) after passing into a function does not give array length: hard to get it wrong once you understand that array (declared on stack) has meta-info that cannot be accessed beyond the stack level it’s initialized.

Loading

Switch between 32-bit and 64-bit user written software like CVX

CVX is a very convenient convex optimization package that allows the user to specify the optimization objective and constraints directly instead of manually manipulating them (by various transformations) into forms that are accepted by commonly available software like quadprog().

What I want to show today is not CVX, but a technique to handle the many different versions of the same program targeted at each system architecture (32/64-bit, Windows/Mac/Linux). Here’s a snapshot of what’s available with cvx:

OS 32/64 mexext Download links
Linux 32-bit mexglx cvx-glx.zip
64-bit mexa64 cvx-a64.zip
Mac 32-bit mexmaci cvx-maci.zip
64-bit mexmaci64 cvx-maci64.zip
Windows 32-bit mexw32 cvx-w32.zip
64-bit mexw64 cvx-w64.zip

You can download all packages for different architectures, but make a folder for each of them by their mexext() name. For example, 32-bit Windows’ implementation can go under /mexw32/cvx. Then you can programmatically initialize the right package for say, your startup.m file:

run( fullfile(yourLibrary, mexext(), 'cvx', 'cvx_startup.m') );

I intentionally put the /[mexext()] above /cvx, not the other way round because if you have many different software packages and want to include them in the path, you can do it in one shot without filtering for the platform names:

addpath( genpath( fullfile(yourLibrary, mexext()) ) );

You can consider using computer(‘arch’) in place of mexext(), but the names are different and you have to name your folders accordingly. For CVX, it happens to go by mexext(), so I naturally used mexext() instead.

Loading

MATLAB Gotchas: Do NOT use getlevels(), getlabels() or categories() for categorical/nominal/ordinal objects Use unique(), unique(cellstr()) instead

I suspect TMW (The MathWorks, maker of MATLAB) hasn’t really thought about dead levels when a categorical object (I mean nominal() and ordinal() as well since they are wrapper child class of categorical()) has elements removed so that some levels doesn’t map to any elements anymore.

For performance reasons, it makes sense to keep the dead levels in because the user can repetitively add and remove the same last level by deleting and adding the same element, causing unnecessary work each time. Naturally, there’s a getlevels()/getlabels()/categories() method in nominal(),ordinal()/categorical() class so you know what raw levels are available. Turns out it’s a horrible idea to expose the raw levels when dead levels are allowed!

Unless you are dealing with the internals of categorical objects, there’s very little reason why one would care or want to know about the dead levels (it’s just a cache for performance). It’s the active levels that are currently mapped to some elements that matters when user make such queries, which is handled correctly by unique().

If there are no dead levels, getlevels() is equivalent to unique(), while categorical() and getlabels() are equivalent to unique(cellstr()), but I’m very likely to run into dead levels because I delete rows of data when I filter by certain criterion.

My first take on it would be to hide getlevels()/getlabels()/categories() from users. But over the years, I’ve grown from a conservative software point of view to accepting more liberal approach, especially after exposure to functional programming ideas. That means I’d rather have a way to know what’s going on inside (keep those functions there), but I’d like to be warned that it’s an evil feature that shouldn’t be used lightly.

Yes, I’m dissing the use of getlevels()/getlabels()/categories() like the infamous eval(). Once in a long while, it might be a legitimate neat approach. But for 99% of the time, it’s a strictly worse solution that causes a lot of damages. It’s way more unlikely that getlevels()/getlabels()/categories() will yield what you really mean with dead levels than multiple inheritance in C++ being the right approach on the first try.

If I use unique() all the time, why would I even bother to talk about getlevels()/getlabels()/categories() since I never used them? It’s because TMW didn’t warn users about the dangers in their documentation. These methods looks legit and innocent, but it’s a usage trap like returning stack pointers in C/C++ (you can technically do it, but with almost 100% certainty, you are telling the computer to do something you don’t mean to, in short: wrong).

I have two encounters that other people using the raw categorical levels that harmed me:

  1. One of my coworkers spoke against upgrading our MATLAB licenses (later withdrew his opposition) because the new versions breaks his old code involving nominal()/ordinal() objects.I was perplexed because it didn’t break any of my code despite I used more nominal() and ordinal() objects than anybody in my vicinity. On close inspection, he was using getlevels() and getlabels() all over the place instead of unique(), which works seamlessly in the new MATLAB.Remember I mentioned that the internal design/implementation details of nominal()/ordinal() changed in MATLAB R2013a? The internal treatment of dead levels has changed. The change was supposed to be irrelevant to end-users by design if getlevels()/getlabels() had not expose dead levels to end-users. Because of the oversight, users have written code that depends on how dead levels are internally handled!
  2. The default factory-shipped grpstats() is still ‘broken’ as of R2015b! If you feed grpstats() with a nominal grouping variable, it will give you lines of NaN because it was programmed to spit out one row for each level (dead or alive) in the grouping variable. Since the dead levels has nothing to group by the reduction function (@mean if not specified), it spits out multiple NaNs as by definition NaN do not equal to anything else, including NaN itself. This is traced to how grp2idx() is used internally: If the grouping variable is a cellstr() or double(), the groups are generated by using unique(), so there are no dead levels whatsoever. But if the grouping variable is a categorical, the developers thought their job is done already and just took it directly from the categorical object’s properties by calling getlabels() and getlevels():
    gidx = double(s);
    ...
    gnames = getlabels(s)';
    glevels = getlevels(s)';

    Apparently the author of the factory-shipped code forgot that there’s a reason why the categorical/unique() has the same function name as double/unique() and cellstr/unique(): the point of overloading is to have the same function name for the same intention! The intention of unique() should be uniformly applied across all the data types applicable. Think twice before relying on language support for type info (like type traits in C++) to switch code when you can use function overloading (MATLAB differentiates by the type of the first argument, C++ looks at the whole signature). A good architecture should lead you to the correct code logic without the need of overriding good practices.

    Rants aside, grpstats() will work as intended if those lines in grp2idx() are changed to:

    gidx = double(s);
    ...
    glevels = unique(s(:));
    gnames = cellstr(glevels);
    

    A higher level fix would be applying grp2idx() to the grouping variable before it was fed into grpstats():

    grpstats(X, grp2idx(g), ...)

    The rationale is that the underlying contents doesn’t matter for grouping variables as long as each of them uniquely stand for the group they represent! In other words, categorical() objects are seen as nothing but a bunch of integers, which can be obtained by casting it to double():

    gidx = double(s);
    grpstats(X, gidx, ...)

    This is what grp2idx() calls under the hood anyway when it sees a categorical. The grp2idx() called from grpstats() will see a bunch of integers, which will correctly apply unique() to them, thus removing all dead levels.

    Of course, use grp2idx() instead of double() because it works across all data types that applies. Why future-constrain yourself when a more generic implementation is already available?

    The sin committed by grpstats() over nominal() is that the variables in glevels and gnames shouldn’t get involved in the first place because they don’t matter and shouldn’t even show up in the outputs. This is what’s fundamentally wrong about it:

    [group,...,ngroups] = mgrp2idx(group,rows);
    ...
    // This code assumes there are no gaps in group levels (gnum), which is not always true.
    for gnum = 1:ngroups
        groups{gnum} = find(group==gnum);
    end

    We can either blame the for-loop for not skipping dead levels, or blame mgrp2idx (a wrapper of grp2idx) for spitting out the dead levels. It doesn’t really matter which way it is. The most important thing is that dead levels were let loose, and nobody in the developer-user chain understand the implications enough to stop the problem from propagating to the final output.

To summarize, the raw levels in categorical objects is a dirty cache including junk you do not want 99.99% of the time. Use unique() to get the meaningful unique levels instead.

Loading

MATLAB Compatibility: nominal() and ordinal() objects since R2013a are not compatible with R2012b and before

In the old days (before R2013a), nominal() and ordinal() were separate parallel classes with astoundingly similar structures. That means there’s a lot of copy-paste-mod going on. TMW improved on it by consolidating the ideas into a new categorical() class, which nominal() and ordinal() derives from it.

The documentation mentioned that nominal() and ordinal() might be deprecated in the future, but I contacted their support urging them not to. It’s not for compatibility reasons: nominal() and ordinal() captures the common use cases that these two ideas do not need to be unified, and the names themselves clearly encodes the intention.

If the user want to exploit the commonalities between the two, either it’s already taken care of by the parent’s public methods, or the object can be sliced to make it happen. I looked into the source code for nominal() and ordinal(): it’s pretty much a wrapper over categorical’s methods yet the interface (input arguments) are much simpler and intuitive because we don’t have to consider all the more general cases.

Back to the titled topic. Because categorical()’s properties (members) are different from pre R2013a’s nominal() and ordinal() objects, the objects created in R2012b or before cannot be loaded correctly in newer versions. That means the backward compatibility is completely broken for nominal()/ordinal() as far as saved objects are concerned.

There’s no good incentive to solve this problem on the TMWs side because the old nominal()/ordinal() is short-lived and they always want everybody to upgrade. Since I use nominal() most of the time and the ones that really need to be saved are all nominal(), I recommend the converting (‘casting’) them to cellstr by

>> A = nominal({'a','a','b','c'});
>> A = cellstr(A)
A = 
    'a'    'a'    'b'    'c'

Remember, nominal() is pretty much compressing a ton of cellstr into a few unique items and mapping the indices. No information is lost going back and forth between cellstr() and nominal(). It’s just a little extra computations for the conversion.

As for ordinal(), I rarely need to save it because order/level assignment is almost the very last thing in the processing chain because it changes so frequently (e.g. how would you draw the lines for six levels of fatness?), I might as well just not save it and reprocess the last step (where the code with ordinal() sits) when I need it.

Nonetheless, if you still want to save ordinals() instead of re-crunching it, this time you’ll want to save it as numerical levels by casting the ordinal() into double():

>> A = ordinal([1 2 3; 3 2 1; 2 1 3],{'low' 'medium' 'high'}, [3 1 2])
A = 
     medium      high        low    
     low         high        medium 
     high        medium      low    
>> D = double(A)
D =
     2     3     1
     1     3     2
     3     2     1
>> U = unique(A)
U = 
     low 
     medium 
     high 
>> L = cellstr(U)
L = 
    'low'
    'medium'
    'high'
>> I = double(U)
I =
     1
     2
     3
>> A_reconstructed = ordinal(D, L, I)
A_reconstructed = 
     medium      high        low    
     low         high        medium 
     high        medium      low

You’ll save (D, L, I) from old MATLAB and load it and reconstruct it with the triplets from the new MATLAB (I’d suggest using structs to keep track of the triplets). I know it’s a hairy mess!

 

Loading

MATLAB Gotchas: Adding whitespace in strcat()

strcat() is a very handy function in MATLAB that allows you to combine strings using a mixture of cellstr() and char strings and it will auto-expand the char strings to match the cellstr() if necessary.

However, by design intention, strcat() removes trailing white spaces by internally applying deblank() to all char string inputs. It does NOT deblank cellstr() inputs. So if you want to combine date and time with a space, you have to use {‘ ‘} instead of ‘ ‘:

date = '2000-01-01';
time = '00:00:01';
>> strcat(date, ' ', time)  % The ' ' is ignored
ans =
2000-01-0100:00:01
>> strcat(date, {' '}, time)  % The ' ' is ignored
ans = 
    '2000-01-01 00:00:01'
>>

I find this more confusing than helpful. Including myself and other users, we naturally resort to processing line by line using cellfun() or other tricks just to get around the deblank() problem without taking a second look at the documentation because

  • rolling our own implementation is marginally as annoying as the deblank()
  • we expect cellstr() to match the dimensions without auto-expanding. I naturally thought it would expand only if it’s a char string.

Well, somebody asked this question on the newsgroup before, so obviously it’s not an intuitive design. It make sense to do it the way MATLAB designed strcat() because we need some way to tell MATLAB whether I want my inputs deblanked or not.

I think it’s more intuitive to have MATLAB’s default strcat() not to deblank() char strings at all and have a strcat_deblanked() that deblanks the inputs before feeding into strcat().

Unfortunately this behavior is there for a long time, so it’s too late to change it without affecting compatibility. Might as well live with it, but this is one of the very few unnatural (or slightly illogical design choice) of MATLAB to keep in mind.

Loading