stan-dev / stanc3

The Stan transpiler (from Stan to C++ and beyond).
BSD 3-Clause "New" or "Revised" License
140 stars 44 forks source link

[BUG] Compiler optimizations can prevent bounds checks #1295

Open WardBrian opened 1 year ago

WardBrian commented 1 year ago

Current Behavior:

Enabling compiler optimizations currently stops emitting a default assignment for a variable if it is immediately assigned to (https://github.com/stan-dev/stanc3/pull/1029). However, this changes the behavior of those very assign functions when it comes to bounds checks:

https://github.com/stan-dev/stan/blob/4ec109a21e5bd1cd43eed724ef286aadec3b5e7a/src/stan/model/indexing/access_helpers.hpp#L58

This means the same model:

parameters {
  vector[3] y;
}

model {
  vector[2] x;
  x = y;
  y ~ std_normal();
  // prevent copy-elision and dead-code-elimination on x
  x[1] = 0;
  print(x);
}

Will throw an error at --O0:

Unrecoverable error evaluating the log probability at the initial value.
Exception: vector assign rows: assigning variable x (2) and right hand side rows (3) must match in size (in '../../ml/stanc3/bounds.stan', line 12, column 4 to column 10)
Exception: vector assign rows: assigning variable x (2) and right hand side rows (3) must match in size (in '../../ml/stanc3/bounds.stan', line 12, column 4 to column 10)

But will sample to completion at --O1.

Expected Behavior:

Bounds checks are applied uniformly.

WardBrian commented 7 months ago

In https://github.com/stan-dev/stan/issues/3271 a user encountered a alternative version of this bug which resulted in an internal error being shown:

Chain 1 Rejecting initial value:
Chain 1   Log probability evaluates to log(0), i.e. negative infinity.
Chain 1   Stan can't start sampling from this initial value.
Chain 1 Iteration:   1 / 1000 [  0%]  (Warmup) 
Chain 1 Exception: In serializer: Storage capacity [763] exceeded while writing value of size [16] from position [759]. This is an internal error, if you see it please report it as an issue on the Stan github repository. (in '/var/folders/3d/czqqm9h550g3ktn68hmtkh080000gn/T/RtmpabL6Uf/model-a7f34d9ee239.stan', line 45, column 4 to column 51)
SteveBronder commented 7 months ago

Talking to @WardBrian we think the best answer here is to have overloads for assign(...) that take in the intended sizes as the last arguments.

WardBrian commented 7 months ago

I think that will work. This case only occurs when there is a plain variable on the LHS (e.g. no indexing), so it shouldn't need to combinatorially interact with the other overloads which handle that.