Fix: Logic for scalar, data matrix, and autodiff matrices

[SteveBronder]

Summary

Fixes #1232 where some mixes of scalars, data matrices, and autodiff matrices would not be promoted to SoA matrices appropriately.

For the following code we would not promote to SoA when we should have

data {
  int N;
}
parameters {
  real alpha;
  vector[N] beta;
}
model{
  vector[N] mu = alpha + rep_vector(0.0, N) + beta;
  target += sum(mu);
}

but for the following model block it does make SoA matrices correctly

model{
  vector[N] mu = alpha + (rep_vector(0.0, N) + beta);
  target += sum(mu);
}

This is because of a bug in the logic for detecting whether we should demote expressions that have a mix of scalars and data matrices.

The difference between the two model blocks above is that you have, in pseudo mir code for the first has a scalar and data matrix operation inside of the expression

assign(mu, add(add(alpha, rep_vector(0.0, N)), beta)

while the second, because of the parenthesis, has a scalar and autodiff matrix operation

assign(mu, add(alpha, add(rep_vector(0.0, N), beta))

The first fails because the optimizer sees a scalar and data matrix operation and fails the whole line. But the second just has a scalar and autodiff matrix operation so it does not fail.

Overall I need to look deeper to understand why I had this particular piece of logic. It was an issue as I was building the optimization, but I forget the exact instance of when this can go badly and need to look into it.

But just checking first in the expression if any side is an autodiff matrix, then checking whether there are scalar / data matrix operations for failing does fix the example model here.

Submission Checklist

• [x] Run unit tests
• Documentation
  • [ ] If a user-facing facing change was made, the documentation PR is here:
  • [x] OR, no user-facing changes were made

Release notes

Fix logic for deducing whether mixes of scalars, data matrices, and autodiff matrices can be promoted to SoA

Copyright and Licensing

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]]

Codecov Report

Merging #1345 (ec37a75) into master (0f36ee7) will decrease coverage by 0.14%. Report is 4 commits behind head on master. The diff coverage is 62.31%.

@@            Coverage Diff             @@
##           master    #1345      +/-   ##
==========================================
- Coverage   89.32%   89.19%   -0.14%     
==========================================
  Files          64       64              
  Lines       10588    10626      +38     
==========================================
+ Hits         9458     9478      +20     
- Misses       1130     1148      +18     

Files Changed	Coverage Δ
src/analysis_and_optimization/Optimize.ml	93.30% <ø> (ø)
src/middle/UnsizedType.ml	72.89% <0.00%> (-7.52%)	:arrow_down:
src/analysis_and_optimization/Memory_patterns.ml	89.06% <87.75%> (+1.01%)	:arrow_up:

... and 1 file with indirect coverage changes

[SteveBronder]

@WardBrian sorry my last fix didn't really tackle the underlying problem. I think I have it now though and will update the summary once the tests pass.

[SteveBronder]

Closing this as I figured out a simpler solution