HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Coordinating camera stage movement and data readout

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
coordinatingcameramovementreadoutstageanddata

Problem

The purpose of this code is important, so here's a quick rundown.

This code runs controls a physical machine which utilizes a camera and a moving stage holding an object to be imaged. Imaging can be broken down into two parts; exposing (collecting photons) and readout (reading the formatted image from the camera over USB).

Stage moves are performed asynchronously. The object being imaged is broken up into many (700+) "tiles" which must be imaged separately, so one move + expose + readout for each tile.

An optimization which saves ~3 minutes per imaging operation is to begin moving to tile N+1 after exposing tile N but before readout has completed. We then wait for readout to complete, expose the current tile, and repeat the process again.

This leads to a loop structure that, while correct, is a bit confusing. It's not a horrible piece of code, but it's been bugging me and I'd like to spend some time today refactoring it. Problem is, I haven't come up with any great ideas.

Here's the loop structure:

void ScanLoopExample()
{
    var tiles = GetAllTiles();
    var firstTile = tiles.First();

    // get into position for the first tile
    MoveStageAsync(firstTile.X, firstTile.Y, firstTile.Z);

    foreach(var nextTile in tiles.Skip(1))
    {
        // wait for the move to complete
        WaitForMotionEnd();

        // expose over the first tile
        ExposeTile();

        // get into position for the next tile
        // we *must* make this move right here in 
        // order to save time moving while the camera is busy
        MoveStageAsync(nextTile.X, nextTile.Y, nextTile.Z);

        // wait for the image to arrive from the camera,
        // we cannot expose again until this completes
        WaitForReadout();
    }

    // because we only moved to the last tile
    // without capturing, capture it here
    WaitForMotionEnd();
    ExposeTile();
    WaitForReadout();
}


Like I said, not the worst code ever, but there are a couple of thin

Solution

Since what matters here is the ordering of the operations, not what the operations mean, I'm going to use these abbreviations:

A - MoveStageAsync
B - WaitForMotionEnd
C - ExposeTile
D - WaitForReadout


(I'm trying to explain how I got my version using these abbreviations, if you find them confusing, skip to the code.)

So with 3 iterations, your code is doing this: ABCADBCADBCD, and the naive, slow version you optimized from did this: ABCDABCDABCD. If we wrap parentheses around the part that is in the loop body, we can abbreviate what your code is doing as A(BCAD)BCD and the unoptimized version as (ABCD). Assuming that we have to keep the same order as what your code is currently doing, we can shift what's in the body of the loop slightly to get AB(CADB)CD and ABC(ADBC)D. There isn't a way to shift the loop all the way to the beginning or end and do things in the same order you're currently doing them in.

If we try to find an ordering that lets us just toss the whole thing in a loop (like (ABCD), only efficient), we have to move to the first location first (nothing else makes sense), we have to wait for the motion to end next (nothing else makes sense, again), then we have to expose before we can wait for the exposure. So the only way to do it with everything in the loop was the original, slow version.

So we have 3 possibilities: A(BCAD)BCD, AB(CADB)CD, and ABC(ADBC)D. If we look at what those 3 would mean, I think AB(CADB)CD is the most readable.

Here it is as code:

MoveStageAsync(firstTile.X, firstTile.Y, firstTile.Z);
WaitForMotionEnd();

foreach(var nextTile in tiles.Skip(1))
{
    ExposeTile();

    MoveStageAsync(nextTile.X, nextTile.Y, nextTile.Z);

    WaitForReadout();
    WaitForMotionEnd();
}

ExposeTile();
WaitForReadout();


There are a few nice things about it: the setup and teardown around the loop both make sense as standalone actions, the waits in the loop body are both next to each other, and the loop body as a whole makes sense as a standalone action. Although the loop index is a tile, it's really a loop over the transitions from one tile to another.

Code Snippets

A - MoveStageAsync
B - WaitForMotionEnd
C - ExposeTile
D - WaitForReadout
MoveStageAsync(firstTile.X, firstTile.Y, firstTile.Z);
WaitForMotionEnd();

foreach(var nextTile in tiles.Skip(1))
{
    ExposeTile();

    MoveStageAsync(nextTile.X, nextTile.Y, nextTile.Z);

    WaitForReadout();
    WaitForMotionEnd();
}

ExposeTile();
WaitForReadout();

Context

StackExchange Code Review Q#25818, answer score: 2

Revisions (0)

No revisions yet.