1.0.3 broke error message line number reporting

[Bob Carpenter]

Something changed in 1.0.3. that is now preventing
Stan from reporting line numbers and column numbers
of errors:

http://code.google.com/p/stan/issues/detail?id=78

I marked it critical and assigned it to myself. I
suspect the problem arose from the changes Marcus made
to error testing.

I'll also try to figure out how to unit test some
of this behavior. I just hate having unit tests depend
on text messages, since they change, but it's better
than nothing and may prevent this kind of gaff on our
part again.

- Bob

[Marcus Brubaker]

Not trying to avoid guilt, but I'm not sure my changes could have done
that. I only touched the code generation bits to spit out try/catch
blocks around the transformed parameter checks, not the actual parser
itself.

If you do construct a unit test for it, git bisect should help you
find the offending change.

Cheers,
Marcus

> --
>
>

[Bob Carpenter]

Now that I'm looking at the code, I think the
problem is what I did to deal with the R
std::cout and std::cerr access issues. Looks like
things are getting caught too early and rethrown
past where I have access to the right iterator.

Shouldn't be a big deal to fix.

I keep telling myself I'll learn more git, but it always
seems easier to just deal with whatever I've got.

- Bob

[Ben Goodrich]

On Thursday, November 1, 2012 3:33:06 PM UTC-4, Bob Carpenter wrote:

I'll also try to figure out how to unit test some
of this behavior.  I just hate having unit tests depend
on text messages, since they change, but it's better
than nothing and may prevent this kind of gaff on our
part again.

Does stanc return 0 when it parses successfully and something else if it fails? If not, it should. At which point you can just test the return value for a lot of things.

Ben

[Bob Carpenter]

Yes, but I don't think that's relevant here. It has
a bunch of non-zero return codes for different kinds
of failures, not that any of them are getting used
anywhere.

The problem is that we're losing the line numbers on
the error reports.

This is all tied up in Boost's crazy iterator scheme.

I've looked further and am now really puzzled as to how
this could've broken at all.

- Bob

[Ben Goodrich]

Maybe we shouldn't worry so much about error codes and just return the line number where it thinks the error is?

[Bob Carpenter]

On 11/1/12 4:26 PM, Ben Goodrich wrote:
> Maybe we shouldn't worry so much about error codes and just return the line number where it thinks the error is?

Maybe the bug report itself will be clearer than
my messages as to what's wrong:

http://code.google.com/p/stan/issues/detail?id=78

This used to work in 1.0.2 and is broken as of 1.0.3.

It has nothing at all to do with returned error codes
at least as far as I can tell.

- Bob

[Ben Goodrich]

FWIW, git bisect points to 54333acd69c8ab5445b8af48edd91732c9c344d1


goodrich@CYBERPOWERPC:/opt/stan$ git bisect start
goodrich@CYBERPOWERPC:/opt/stan$ git bisect good v1.0.2
goodrich@CYBERPOWERPC:/opt/stan$ git bisect bad v1.0.3
Bisecting: 101 revisions left to test after this (roughly 7 steps)
[ac898765c412f57b5e18db25689092e0b091be5a] added line number to-do item for debugging
goodrich@CYBERPOWERPC:/opt/stan$ git bisect good
Bisecting: 50 revisions left to test after this (roughly 6 steps)
[6ab3c06fa56caaca10dcf0e16f7c5ca784f185d0] add diff.sh for diffing manual
goodrich@CYBERPOWERPC:/opt/stan$ git bisect good
Bisecting: 25 revisions left to test after this (roughly 5 steps)
[111563ce698d6d89cfc2cf9f426f8d215378a4fe] Merge branch 'master' of https://code.google.com/p/stan
goodrich@CYBERPOWERPC:/opt/stan$ git bisect bad
Bisecting: 12 revisions left to test after this (roughly 4 steps)
[c42d0254f2d5b670dc6cac944acaee7c5f13669e] Merge branch 'master' of https://code.google.com/p/stan
goodrich@CYBERPOWERPC:/opt/stan$ git bisect bad
Bisecting: 5 revisions left to test after this (roughly 3 steps)
[d5fa7b71ebe9915ed0aff8d5b9e16fac7b24c3de] Merge branch 'master' of https://code.google.com/p/stan
goodrich@CYBERPOWERPC:/opt/stan$ git bisect good
Bisecting: 2 revisions left to test after this (roughly 2 steps)
[a4e7d6778805189c9703ea3397d836d2a233fc7d] added error message dump if any warnings to command
goodrich@CYBERPOWERPC:/opt/stan$ git bisect good
Bisecting: 0 revisions left to test after this (roughly 1 step)
[54333acd69c8ab5445b8af48edd91732c9c344d1] added prints for warnings to parser code
goodrich@CYBERPOWERPC:/opt/stan$ git bisect bad
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[875d488ae4deda3b2d924a4195831ec16f3fc5e0] removed assert() and replaced with exceptions
goodrich@CYBERPOWERPC:/opt/stan$ git bisect good
54333acd69c8ab5445b8af48edd91732c9c344d1 is the first bad commit
commit 54333acd69c8ab5445b8af48edd91732c9c344d1
Author: Bob Carpenter <carp@alias-i.com>
Date:   Wed Oct 24 15:36:32 2012 -0400

    added prints for warnings to parser code

:040000 040000 57cc98ccf37b74408f4a779d25e60821d87ca144 8753056a825a9ad1268313060f9a64e061c3d38a M      src

Ben

[Ben Goodrich]

On Thursday, November 1, 2012 4:51:50 PM UTC-4, Ben Goodrich wrote:

FWIW, git bisect points to 54333acd69c8ab5445b8af48edd91732c9c344d1


My "test" in git bisect was just doing

make clean-all && make -j8 CC=clang++ /tmp/SU/parser/normal

and looking to see if the error is found at line=9, column=3

Ben

[Bob Carpenter]

Groovy.

Did you look at the test by hand or write some code?

I didn't see that in the example.

- Bob

Author: Bob Carpenter <ca...@alias-i.com>

[Ben Goodrich]

I was just eyeballing the parser output to judge between good and bad states. But git bisect can be scripted if you have a conclusive way of testing. In this case, my unwritten script would do

make clean-all && make -j8 CC=clang++ /tmp/SU/parser/normal 2>/tmp/parser.log
grep "line=9" /tmp/parser.log

and a successful return code of grep would define the git bisect good state.

Ben