Check for closure of data?

[jeanluct]

When computing the entropy of taffy pullers, based on rod trajectories, I found that it's easy to assume a set of trajectories is closed, when it really isn't because of some bug. Would it be safer to have colorbraiding check for this? If the trajectories don't close, then colorbraiding would issue an error recommending the user call closure first.

Could this break things? I've implemented it but haven't pushed yet.

[jeanluct]

This the code I added to colorbraiding for now (in branch iss130-check-for-closure):

% Check if the final points are close enough to the initial points (setwise).
% Otherwise this could be an error with the user's data.
% Force them to call 'closure(XY)' first.
XYstart = squeeze(XY(1,:,:)).';
XYend = sortrows(squeeze(XY(end,:,:)).');
if any(sqrt(sum((XYstart - XYend).^2,2)) > delta)
  error('BRAIDLAB:braid:colorbraiding:notclosed',...
        ['The trajectories do not form a closed braid. ' ...
         'Call ''closure'' on the data first.']);
end

Note that the testsuite is broken by this, but that's an easy fix.

[mbudisic]

Wait, what about when trajectories are genuinely not closed, like in Aref vortex? databraid still ends up invoking colorbraiding, doesn't it?

[jeanluct]

Ok, maybe this does create a problem with databraid. The closure command doesn't really work well on time-indexed data, since it would need to append an additional time. If we just duplicate the final time then the databraid constructor will complain.

Possible solution: allow non-closed data for databraid?

[mbudisic]

I'm not really sure, currently databraid inherits the braid. From perspective of OOP, "databraid" is a "braid", so if "braid" cannot have something, neither should "databraid". I guess the OOPway would be to reverse the relationship - make braid (a more specific thing) a subclass of databraid (more general thing).

But that's a pain to reverse.

Perhaps colorbraiding could have a flag, like "forceClosed" or something like that, which would make it check for closure. Or you could just check for closure outside braidlab, not forcing another user to follow in your steps?

[jeanluct]

I implemented something like your suggestions: colorbraiding now has a testclosure flag. The inheritance relationship doesn't bother me too much. After all, databraids are not really braids either because of the extra time info. The best would be for both to inherit from some abstract class, but that sounds like too much work. Maybe when rewriting braidlab in Python...

I really think we should implement this since it uncovered three bad bugs in my taffy puller codes. All a user has to do to get around it, if they really want to, is replace braid(XY) by braid(closure(XY)).

[jeanluct]

BTW, the change only required changing 3 lines in the testsuite, all in braidTest. I think we're good to go.

Well, one last thing: instead of an error we could consider making this only a warning?

[mbudisic]

OK, fair enough - just turn the testclosure off in databraid invocation of colorbraiding.
Perhaps a warning would be better after all, e.g., if you don't care that the braid doesn't close, you just want a sequence of generators?

[jeanluct]

Right, I think we'll go with a warning. It also makes the example in the guide easier to adapt.

[jeanluct]

Branch merged in e49d3d4.

[jeanluct]

Oops, the code snippet above fails for some cases: when two X coordinates are very close together (within tolerance), but get sorted differently at the beginning and the end.

The best way to solve this is to use assignmentoptimal. That function was in the private folder of +braidlab, which is inaccessible to braid because of the weird Matlab scoping. So I moved assignmentoptimal to +util instead. This required shuffling Makefiles around. Hopefully it's ok now.

See 2f2c4a1.