New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C++ exceptions from generated quantities cause shorter chain lengths #2568

Closed
syclik opened this Issue Jul 5, 2018 · 8 comments

Comments

Projects
None yet
4 participants
@syclik
Member

syclik commented Jul 5, 2018

Summary:

C++ exceptions that come from the generated quantity blocks are not handled properly. This causes short chain lengths.

Reproducible Steps:

Here's a small Stan program that shows the issue:

parameters {
  real alpha;
}
model {
  alpha ~ normal(0, 1);
}
generated quantities {
  real<lower = -0.3> beta;
  beta = alpha;
}

Current Output:

In CmdStan, the number of rows in the output file is short. I see a lot of these messages:

Exception: foo_model_namespace::write_array: beta is -0.741171, but must be greater than or equal to -0.3  (in 'foo.stan' at line 8)

which are different than when there is an exception in other parts of the code:

Informational Message: The current Metropolis proposal is about to be rejected because of the following issue:
Exception: ...

Expected Output:

I except there to still be the number of draws specified.

Current Version:

v2.17.1

@bob-carpenter

This comment has been minimized.

Contributor

bob-carpenter commented Jul 5, 2018

The issue needs to be updated to specify the intended fix. I see two three options:

  1. Reject the entire draw retroactively if generated quantities fails.

  2. Report NaN for all values when there is an error in generated quantities.

  3. Abort the Stan run.

(1) is problematic. because we've drawn the paramter values according to NUTS, and only then run into a problem. So if we don't keep the draw according to NUTS, we're not appropriately drawing from the posterior. The generated quantities don't affect the posterior. I guess we could somehow make this salvageable by making this a rejection like in transformed parameter constraint violations.

(2) draws from the posterior as specified by the model block.

(3) is what we used to do, reasoning that the errors were unrecoverable after the draw has been taken according to NUTS and the generated quantities fails.

What happens when there's an indexing error? Do we abort for that? If so, we'd do that in generated quantities, too, right?

@seantalts seantalts added this to the 2.17.1++ milestone Jul 6, 2018

@seantalts seantalts added the bug label Jul 6, 2018

@seantalts

This comment has been minimized.

Member

seantalts commented Jul 6, 2018

Same for this one - @syclik has said this is blocking 2.18 so I want to make sure someone is assigned to it and working on it.

@syclik

This comment has been minimized.

Member

syclik commented Jul 6, 2018

@bob-carpenter

This comment has been minimized.

Contributor

bob-carpenter commented Jul 6, 2018

@bgoodri

This comment has been minimized.

Contributor

bgoodri commented Jul 6, 2018

@syclik syclik self-assigned this Jul 7, 2018

@syclik

This comment has been minimized.

Member

syclik commented Jul 7, 2018

@bgoodri, can you unpack your last comment? Does that mean that RStan is expecting all the numbers to be not NaN? If so, what happens when it's NaN?

@bgoodri

This comment has been minimized.

Contributor

bgoodri commented Jul 7, 2018

@syclik

This comment has been minimized.

Member

syclik commented Jul 9, 2018

bob-carpenter added a commit that referenced this issue Jul 9, 2018

Merge pull request #2571 from stan-dev/bugfix/issue-2568-exceptions-gq
Fixes #2568. Updates the code so on exception in generated quantities, all generated quantities are NaN
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment