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

Better error for unsupported type in reduce_sum #1332

Closed nhuurre closed 1 year ago

nhuurre commented 1 year ago

Submission Checklist

reduce_sum documentation says the array being sliced can contain any type but that's not really true. If you try to use complex number you get a convoluted error.

functions {
  real my_func(array[] complex y_slice, int start, int end) {
    return std_normal_lpdf(get_real(y_slice) |) + std_normal_lpdf(get_imag(y_slice) |);
  }
}
parameters {
  array[10] complex a;
}
model {
  target += reduce_sum(my_func, a, 1);
}
Semantic error in 'example.stan', line 10, column 12 to column 37:
   -------------------------------------------------
     8:  }
     9:  model {
    10:    target += reduce_sum(my_func, a, 1);
                     ^
    11:  }
   -------------------------------------------------

Ill-typed arguments supplied to function 'reduce_sum':
(<F1>, array[] complex, int)
where F1 = (array[] complex, int, int) => real
Available signatures:
(<F2>, array[] real, int) => real
where F2 = (array[] real, data int, data int) => real
  The first argument must be
   (array[] real, data int, data int) => real
  but got
   (array[] complex, int, int) => real
  These are not compatible because:
    The types for the first argument are incompatible: one is
     array[] complex
    but the other is
     array[] real

Despite the error saying that only array[] real is allowed, reduce_sum does work with arrays of vectors and matrices as well; it's only the complex number types that are not supported in the math library.

This PR adds a new error case that explains the limitations more precisely.

Copyright and Licensing

Copyright holder: Niko Huurre By submitting this pull request, the copyright holder is agreeing to license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)

codecov[bot] commented 1 year ago

Codecov Report

Merging #1332 (1a77d77) into master (5ae292a) will decrease coverage by 0.04%. The diff coverage is 83.33%.

@@            Coverage Diff             @@
##           master    #1332      +/-   ##
==========================================
- Coverage   88.80%   88.76%   -0.04%     
==========================================
  Files          64       64              
  Lines        9860     9865       +5     
==========================================
+ Hits         8756     8757       +1     
- Misses       1104     1108       +4     
Impacted Files Coverage Δ
src/middle/Stan_math_signatures.ml 98.04% <ø> (-0.02%) :arrow_down:
src/frontend/Semantic_error.ml 92.74% <75.00%> (-0.77%) :arrow_down:
src/frontend/Typechecker.ml 89.69% <87.50%> (-0.19%) :arrow_down: