Changes to function signatures.

[Daniel Lee]

So far, we've changed:
- nan to not_a_number
- infinity to positive_infinity

Additional change:
- log_loss to binary_log_loss.

I'll keep responding to this thread for additional changes (if any).
Please let me know if you have any objections.



Daniel

[Daniel Lee]

I ran into some issues with the negation prefix operators.

Bob, if you have a free minute, can you look at
src/test/models/function_signatures.stan? Uncommenting line 408 causes
problems. I don't think it's finding the minus() function defined in
src/stan/agrad/matrix.hpp.



Daniel

[Bob Carpenter]

On 9/6/12 10:35 PM, Daniel Lee wrote:
> I ran into some issues with the negation prefix operators.
>
> Bob, if you have a free minute, can you look at
> src/test/models/function_signatures.stan? Uncommenting line 408 causes
> problems. I don't think it's finding the minus() function defined in
> src/stan/agrad/matrix.hpp.

Sure, but you need to push it, first.

- Bob

[Daniel Lee]

Everything I have is pushed... at least I think it is.

> --
>
>

[Daniel Lee]

Odd... we have unit tests in place, so the functions themselves work.
It's just an issue in the generated code.

[Bob Carpenter]

Your test works fine on my machine, but it still looks
like a no-op to me:

-----------------
#include <gtest/gtest.h>

TEST(ModelFunctionSignatures, Compiles) {
SUCCEED()
<< "This test is run if and only if function_signatures.stan is compiled successfully.";
}
------------------

- Bob

[Daniel Lee]

That was after uncommenting out line 408? This is the line in question:

transformed_param_vector <- -d_vector;

> --
>
>

[Bob Carpenter]

Something tells me we're on a different page.
There is no line 408. The following is all that's
in the current repository version:

src/test/models/function_signatures_test.cpp

-----------------
#include <gtest/gtest.h>
TEST(ModelFunctionSignatures, Compiles) {
SUCCEED()
<< "This test is run if and only if function_signatures.stan is compiled
successfully.";
}
------------------

- Bob

[Daniel Lee]

Sorry, the stan file:

src/models/function_signatures.stan

> --
>
>

[Bob Carpenter]

Thanks. I didn't read closely enough.
I wasn't looking for tests in

src/models

I've checked the unit test models in under

test/gm/model_specs

and I think this is where this one should go.
I want the src/models to be example models users
can look at. This one's purely for testing.

- Bob

[Daniel Lee]

Ok. Sounds good.

--

[Bob Carpenter]

As is, Stan doesn't support assignment of DATA matrices,
vectors, arrays, to PARAMETER matrices, vectors, arrays.
The issue Daniel found is that the parser accepts it, but
C++ won't compile the result.

Because we want everything that's parsable by Stanc to
be compilable by C++, this has to be fixed. I outline the
problem and suggest two fixes below. Any opinions?

The instances that are problematic are:

{ transformed param var,
local var in transformed param,
local var in model }
<-
{ data var,
transformed data var,
constant* }

* We don't actually have any non-scalar constants, but if we
did, they would pose a problem.

There are no parameters in scope when the transformed
data block runs, so the reverse assignment, which is also
legal in C++, never comes up in Stan.

Fix 1. Disallow the assignment in the parser and
add a description to the doc.

I'm OK with this fix, but it's going to be a bit
complex to implement. I think I'll have to either
(a) propagate var/non-var info along with other type
info in Stan type inference, or (b) walk down the
expression tree corresponding to the right-hand-side
of expressions looking for variables.

Fix 2. Wrap the constant in an auto-dif variable.

This is OK semantically. And it's the easiest to
implement. It shouldn't affect efficiency of param to param
assignment because I'll just wrap the right-hand-side of
assignments to transformed parameters and local
vars in transformed parameters and the model block with
a to_var() the way we do in the matrix lib.

I'm inclined to go with this option, letting the user
write inefficient models rather than having them struggle
with inscrutable edge conditions of the assignment syntax.

- Bob

[Marcus Brubaker]

So to be clear, the issue is that:

data {
matrix[4,5] A;
matrix[4,5] B;
}

model {
matrix[4,5] C;
C <- A - B;
}

will parse fine but won't compile? Also, I assume this is specific to
matrix/vector/array types because I'm sure C[i,j] <- 1.0 would work.
The underlying issue being that the compiler doesn't know how to
convert from, e.g., Eigen::Matrix<double> to Eigen::Matrix<var>.

At any rate, I vote for fix 2. Fix 1 would create an annoying corner
case for people to deal with if we made it illegal. My only concern
is the performance hit, but it should be basically free when it's not
necessary.

Cheers,
Marcus

> --
>
>

[Bob Carpenter]

On 9/7/12 2:37 PM, Marcus Brubaker wrote:
> So to be clear, the issue is that:
>
> data {
> matrix[4,5] A;
> matrix[4,5] B;
> }
>
> model {
> matrix[4,5] C;
> C <- A - B;
> }

Exactly.

> will parse fine but won't compile? Also, I assume this is specific to
> matrix/vector/array types because I'm sure C[i,j] <- 1.0 would work.
> The underlying issue being that the compiler doesn't know how to
> convert from, e.g., Eigen::Matrix<double> to Eigen::Matrix<var>.
>
> At any rate, I vote for fix 2. Fix 1 would create an annoying corner
> case for people to deal with if we made it illegal.

Right.

Even though it's probably not a good idea to do it,
making it illegal would presumably be very confusing.

> My only concern
> is the performance hit, but it should be basically free when it's not
> necessary.

Presumably the compiler would eliminate the no-op wrapper
converting a variable-based matrix to a variable-based matrix.

- Bob

[Daniel Lee]

I moved function_signatures.stan to src/test/gm/model_specs/. I've
also added a test to the existing gm/parser_test.cpp checking that
function_signatures.stan is parsable.

I've added a sort of end-to-end test in
src/test/gm/compile_models_test.cpp. Let me know if we shouldn't have
that in our unit tests. The test itself does nothing, but I've added
explicit dependencies in the makefile that will build bin/stanc, the
generated cpp code for function_signatures.stan, and verifies that the
generated cpp compiles. If anyone wants me to back it out, let me know
and I can pull out the test (and the modifications to the makefile).



Daniel

[Bob Carpenter]

That sound sgreat.

On 9/7/12 7:09 PM, Daniel Lee wrote:
> On Fri, Sep 7, 2012 at 1:25 PM, Bob Carpenter <ca...@alias-i.com> wrote:
>
> I moved function_signatures.stan to src/test/gm/model_specs/. I've
> also added a test to the existing gm/parser_test.cpp checking that
> function_signatures.stan is parsable.
>
> I've added a sort of end-to-end test in
> src/test/gm/compile_models_test.cpp. Let me know if we shouldn't have
> that in our unit tests.

I think we should have it -- I just didn't know
how to do it through make.

- Bob