Avoid overflow in braid.complexity

jeanluct / braidlab

Matlab package for analyzing data using braids

GNU General Public License v3.0

23 stars 9 forks source link

Avoid overflow in braid.complexity #69

Closed jeanluct closed 9 years ago

jeanluct commented 9 years ago

This happens for Marko for large entropy braids.

Possible solutions:

rewrite entropy_helper.cpp to use loop.minlength (rewritten in C++) rather than norm. Forget norm entirely. Then braid.complexity will call braid.entropy. Is there a reason to retain the option to use different measuring functions (intaxis, minlength, norm)? Check speed, to make sure that it doesn't slow us down too much.
switch to VPI in braid.complexity. This will be be sloooooow. We'll need to take a log, but the number of digits should be sufficient for getting double-precision.

The first solution sounds slightly better to us?

jeanluct commented 9 years ago

From Marko Budisic on 2014-10-08 15:51:40+00:00

I'm working on entropy_helper right now: is there a compelling reason to keep 1-indexed arrays within C++? Working inside C++, this is very unintuitive: it took me 15 minutes to debug the simplest test case.

jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-10-08 15:59:17+00:00

No, not really, except it made it easier to port the Matlab code to C++ initially. Why is it so unintuitive? I guess I learned that trick from Numerical Recipes in C, where it's used everywhere.

jeanluct commented 9 years ago

From Marko Budisic on 2014-10-08 16:01:09+00:00

It is unintuitive because C++ uses 0-indexing by default, so when you're writing C++ code and see "const T* a" declaration, you don't assume it's 1-based indexing.

I'll accommodate for both though, instead of re-writing update_rules, I just didn't see it coming.

jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-10-08 16:03:29+00:00

Sorry, it does say "make 1-indexed arrays" as a comment. Does this interfere with your code a lot?

jeanluct commented 9 years ago

From Marko Budisic on 2014-10-08 16:07:28+00:00

It does, when a, b, are constructed, but not in documentation of l2norm. I'll add the clarification.

No, it doesn't interfere at all, I was just wondering if there is a specific reason why 1-indexing is used, since I don't remember other cpp files in braidlab using them (and I used 0-indexing in colorbraiding).

jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-10-08 16:13:28+00:00

If you want to rewrite it to use 0-index that's fine also!

jeanluct commented 9 years ago

From Marko Budisic on 2014-10-08 16:14:01+00:00

Not for now, I took one look at update_rules and decided it's good to support both :)

jeanluct commented 9 years ago

From Marko Budisic on 2014-10-09 00:15:53+00:00

OK, I created @loop/private/loop_helper.hpp which has a computation of loop length using a single for-loop without explicit computation of intersection numbers in between. I don't have the formula typed up yet, but here are my calculations for how it works.

jeanluct commented 9 years ago

From Jean-Luc Thiffeault on 2014-10-09 11:50:02+00:00

Cool. Does this mean we don't get a C++ version of intersec?

jeanluct commented 9 years ago

From Marko Budisic on 2014-10-28 20:36:11+00:00

OK, this appears to be done now - basic tests hold. I'll go and see how the actual research code performs, but I'm close to closing this issue...

jeanluct commented 9 years ago

This seems resolved to me.