probmods / webppl

Probabilistic programming for the web
http://webppl.org
Other
613 stars 89 forks source link

Stack traces #314

Open stuhlmueller opened 8 years ago

stuhlmueller commented 8 years ago

We'd like to have full stack traces (tracebacks) when errors happen, going beyond immediate error locations (#292). This is challenging, since the CPS transform eliminates the stack that is usually used for tracebacks.

In CPS, the stack is represented explicitly using the continuation function. One could try to approach this issue by associating extra information with this function, such as the file and location where it is defined and a pointer to the continuation function that corresponds to the next stack frame. I don't know how difficult it would be to do the latter.

ngoodman commented 8 years ago

Alternatively, perhaps the trampoline function could be responsible for keeping around a stack trace: each time it is called it pushes the current location, on return it pops. If this incurs a lot of overhead we could have a debug mode where the tracing trampoline is used.

stuhlmueller commented 8 years ago

Yes, it could make sense for the trampoline runner to keep around a stack. It's only called once itself (apart from a few exceptional circumstances), so the modification of the stack would have to originate elsewhere. We could extend the code built by the trampolining transform such that it adds to the stack in places that previously were compound function calls, and pops in places that were return statements. (This may be what you meant.)

stuhlmueller commented 8 years ago

Do we have a plan for this? If it's not clear how to do it, we can push it to 1.0 or later.

null-a commented 8 years ago

Do we have a plan for this?

I don't. Let's discuss it once 0.8 is out of the way.

longouyang commented 8 years ago

Would it be enough for the trampoline runner to keep track of the current address? (i.e., can we always map addresses back to locations in the user code or the header?)

If so, I think we'd have to keep track of s, k, and a in the runner -- I'm not sure if this would break anything else.

null-a commented 8 years ago

Assorted thoughts on this:

perhaps the trampoline function could be responsible for keeping around a stack trace: each time it is called it pushes the current location, on return it pops.

We could extend the code built by the trampolining transform such that it adds to the stack in places that previously were compound function calls, and pops in places that were return statements.

I'm still a little hazy on exactly what this would look like, but it seems to me that just maintaining a single stack somewhere won't be sufficient. When we invoke a continuation we may need to effectively replace the current stack with an entirely different one, and if we're only maintaining a single stack I'm not sure where the information to do that comes from? Am I missing something?

One could try to approach this issue by associating extra information with this function, such as the file and location where it is defined and a pointer to the continuation function that corresponds to the next stack frame. I don't know how difficult it would be to do the latter.

I wonder whether something like this would require us to hook up these pointers manually in our hand written cps code? If so, that sounds fiddly.

Would it be enough for the trampoline runner to keep track of the current address

I've been wondering about this too -- it's an appealing approach to explore I think. I've only got as far as imagining that when entering a function we could stick the current address in a global variable so that when we encounter an exception we can easily grab the most recent address and process it as required. Avoiding the global would be good, maybe via the runner or maybe some other way. (If we can avoid tangling this up with the runner too much it would be great.)

i.e., can we always map addresses back to locations in the user code or the header?)

We don't have easy access to this at the moment, but I imagine we'd be able to build such a map during the naming transform. There are a few places where the address is extended by hand, but even if we can't include those this would still be useful I think.

ngoodman commented 8 years ago

before doing something hard here, i think it's worth asking ourselves what we want the stack for?

for instance, in my opinion having full stack traces helps debugging only slightly more than having the most recent location in user source (so you know where to look for syntax errors; dynamic bugs you are better off using debug statements to step through anyway). we currently almost have this, but sometimes get a pointer to the header.... so it'd be enough for that to keep track of the most recent user source location -- which is much easier.

anyone have pressing use cases for full stacks?

longouyang commented 8 years ago

Leaving this here for a paper trail: in today's PPAML summer school meeting, we agreed that addresses are essentially stacks and we should try the approach I suggested above.

null-a commented 8 years ago

I've taken a closer look at the idea of using addresses for this. Assuming i didn't mess up, the summary goes like this:

If we combine the information from the current webppl address with the top-most entry of the JS stack trace then I think we have enough information to build a (coarse-grained) stack trace or implement #518.

However, the information we recover from the webppl address isn't sufficient by itself to do this. This is a problem because we were hoping that we could use this to side-step issues we have getting useful stack traces out of some browsers.

As an example, consider the following:

var obj = {f: undefined};
var f = function(x) { obj.f(); };
var g = function(x) { return f(1); };
g(0);

This will throw an error when we evaluate obj.f(), since undefined is not a function.

That program compiles to something like the following (dropping store passing, trampoline thunks...):

var obj = {f: undefined};

var f = function f(_k2, _address134, x) {
  return _k2(obj.f());
};

var g = function g(_k1, _address135, x) {
  return f(_k1, _address135.concat('_130'), 1);
};

g(_k0, ''.concat('_131'), 0);

When we evaluate obj.f() the current address (the value of _address134) will be '_131_130'. By mapping each fragment of the address back to the call site at which it was added to the address, we can use that to reconstruct a stack of function calls leading up to the error. Something like:

f(1)    '_130'
g(0)    '_131'

... which is neat, but it doesn't tell us where we are when the error occurs.

In the special case where obj.f is a webppl function (rather than a primitive JS function) we might be able to take a peek at the address that would be in-place had the call succeeded and get something that works. I can't see how to make it work in general though.

Anyone see anything I'm missing?

longouyang commented 8 years ago

This is probably naive, but doesn't the fact that the top address is _130 tell us that we're inside f when the error occurs?

longouyang commented 8 years ago

Oh - is the issue that, at error-time, we don't have access to the relevant address variable?

If that's the issue, would it be too ugly to update a global currentAddress variable?

null-a commented 8 years ago

is the issue that, at error-time, we don't have access to the relevant address variable?

No, the example above assumes we have access to _address134 when the error occurs. That's where '_131_130' comes from.

This is probably naive, but doesn't the fact that the top address is _130 tell us that we're inside f when the error occurs?

I think it tells us we most recently applied a function bound to the variable f, but not what the value of f was.

(Because at compile time we can build a map from fragments of the address (such as '_130') to the ast node representing a function application (e.g. of f), but we don't know anything about its value at runtime.)

Even if I've messed up, and we can leverage stack addresses to figure out which function we're in, there's still a question about whether having such coarse-grained information is sufficient. If an error occurs in the middle of a really long function, would we be happy if all we can do is show the message text from the JS error, and point to the giant function body and say "it happened somewhere in there"?

None of this is to say that we can't make webppl stack traces or #518 work without accurate JS stack traces. But at the moment, I'm not seeing how to get everything we want by building on only webppl addresses. I'll keep thinking...

longouyang commented 8 years ago

I think it tells us we most recently applied a function bound to the variable f, but not what the value of f was.

I'm not seeing why it's important to know the value of f. In your understanding, what does useful information about f's value look like?

Even if I've messed up, and we can leverage stack addresses to figure out which function we're in, there's still a question about whether having such coarse-grained information is sufficient. If an error occurs in the middle of a really long function, would we be happy if all we can do is show the message text from the JS error, and point to the giant function body and say "it happened somewhere in there"?

This seems better than what we have right now though -- e.g., we currently can't do error highlighting for map(f, xs) if f is undefined but it seems like having addresses would let us do this?

null-a commented 8 years ago

I'm not seeing why it's important to know the value of f.

I've tweaked the example and added a bit more detail in this gist -- see what you think.

This seems better than what we have right now though

I'll come back to this when I get round to posting a status/progress update if that's ok.

longouyang commented 8 years ago

That gist clarifies things, thanks.

Playing around a little, we seem to have more addressing information if we're calling a non-member function:

var bar; // undefined
var f = function(x) { bar(); };
var g = function(x) { return f(1); };
g(0);
null-a commented 8 years ago

This seems better than what we have right now though

I'll come back to this when I get round to posting a status/progress update if that's ok.

If we were to use only stack addresses (and not the information from the JS stack trace) then I think some situations are improved and others are made worse. We can do better if we combine the information from both sources (when available), which is what I've been working on.

null-a commented 8 years ago

I have a branch that contains what I hope is a working implementation of this. The general strategy is to track keep track of the current webppl address at runtime (implemented via an extra transform) and combine the information from that with information from the JS stack trace when an exception occurs. This gives us enough information to recover what might reasonably be called a webppl stack trace.

Here's what it currently looks like in use. This program:

var f = function() {
  map('func', [1,2,3])
}
f();

Shows the following error in the console:

TypeError: fn is not a function
    at header.wppl:260

259| var map = function(fn, l) {
260|   return map_helper(0, l.length - 1, function(i) { return fn(l[i]) })
     ----------------------------------------------------------^
261| }

    at test.wppl:2

1| var f = function() {
2|  map('func', [1,2,3])
   -^
3| }

It's possible to disable the extra transform using the --no-debug flag, yielding behavior which is very similar to what we currently have on dev:

TypeError: fn is not a function
    at header.wppl:260

259| var map = function(fn, l) {
260|   return map_helper(0, l.length - 1, function(i) { return fn(l[i]) })
     ----------------------------------------------------------^
261| }

The example above is using node of course, but all the infrastructure bits were written with the browser in mind. I expect this will allow us to get error highlighting working as desired under Chrome, at least.

@longouyang I'll find somewhere more appropriate than this issue and pick up the question of what needs to happen to integrate this with the editor shortly.

dritchie commented 7 years ago

Did anything ever happen with this?

I'm asking because I'm running into situations with one of my current projects where stack traces would be really useful. Here's an example error:

TypeError: Cannot read property 'sub' of undefined
    at /Users/dritchie/Git/webppl-fork/node_modules/adnn/ad/functions.js:50

49|         sub: OutputType === Tensor ?
50|             function(x, y) { return x.sub(y); } :
    ----------------------------^
51|             function(x, y) { return x - y; },

    at /Users/dritchie/Git/procmod-learn/experiments/ldpbg_fig2/model.wppl:28

27|     var prms = predictedDiscreteParams(params_whichPrim(absID), isTraining);
28|     var dist = predictedDist(absID, Discrete({ps: prms}));
    ------------^
29|     var id = observed(dist, obsNode && obsNode.primitive.id);

This is time-consuming to debug because there are a large number of function calls between the predictedDist call which occurs in the .wppl file being run, and the actual line where the error occurs in adnn. If full webppl stack traces are too difficult to implement, I would love to even have access to the full Javascript stack trace (i.e. the stack trace for all regular javascript code that was called from webppl code--this should just be available, right, without having to keep track of / reconstruct it?)

null-a commented 7 years ago

Did anything ever happen with this?

Yes, we decided to implement a special case of full stack traces, where we show the top frame and the top most frame that points to webppl code. (The error message you pasted is an example of this.) IIRC, prior to this we wouldn't have known that the error originated from line 28 of model.wppl.

If full webppl stack traces are too difficult to implement

Possibly not. I think I had to do pretty much all of the hard stuff to get the special case working. (Though I'd have to go back and check to be sure.)

I would love to even have access to the full Javascript stack trace

More often than not the JS stack trace is no help at all (as you know), but we can easily print them if people don't mind the noise? (Or maybe we hide this behind a command line arg if they are useful only rarely?)

dritchie commented 7 years ago

I think I was a bit unclear in my request--I'd be interested in the part of the stack that corresponds to JS code (i.e. not webppl code) that occurs between the two locations printed in the error above. My intuition (but maybe I'm wrong--I haven't thought about this nearly as much as you have) is that this would be easier to get at than the full stack trace including webppl code frames, b/c there's nothing to reconstruct due to CPS--IIUC, those JS frames should just be there.

null-a commented 7 years ago

I'm confused... The JS stack trace is still around and we can easily get at it. See e.g. the example of printing this that I linked in my previous comment. It seems like this will contain the stuff you're interested in (if v8 is set to capture enough frames) albeit with a few extra bits too. Would printing this (in addition to what we print now) not meet your needs?

dritchie commented 7 years ago

Ah, yep, that looks like it'd do the trick (plus some extra cruft).

JsonFreeman commented 7 years ago
Error: All paths explored by Enumerate have probability zero.
    at /usr/local/lib/node_modules/webppl/src/inference/enumerate.js:86

85|             if (ad.scalar.peq(this.marginal.size, 0)) {
86|                 throw new Error('All paths explored by Enumerate have probability zero.');
    ----------------------^
87|             }

    at qaExperiments/qaAndOrNeg1/qaAndOrNeg1.wppl:55

54|         display(arguments)
55|         return apply(generateDataModel.Q2, [q]);
    ---------------^
56|     }, 1);

I believe this is a case where it would be very useful to have a full stack trace. Or at least the the deepest webppl stack frame.