Fix code generation issue with overloaded distributions in ~ statements
#1466
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1466 +/- ##
==========================================
- Coverage 89.82% 89.76% -0.07%
==========================================
Files 63 63
Lines 10598 10613 +15
==========================================
+ Hits 9520 9527 +7
- Misses 1078 1086 +8
|
There was a problem hiding this comment.
(it always bothered me that FnLpdf lumped these two together, anyway)
They're lumpled together because, as far as the backend is concerned, they behave identically. I'm not sure if having different names in the frontend was a good idea.
Technically, you don't need a new Fun_kind variant here either--you could decide which suffix to use based on UnsizedType.is_discrete_type arg.
| The first argument must be a pure function but got a probability density or mass function. These function types are not compatible. | ||
| The first argument must be a pure function but got a probability density function. These function types are not compatible. |
There was a problem hiding this comment.
This looks nicer, though.
Certainly up for debate. @bob-carpenter agrees, for what it's worth, but it's probably not worth changing now
True -- and my first attempt used only that, until I realized that we also need to differentiate between StanLib and UserDefined. If you prefer that solution I can marry the two |
Initially reported in https://discourse.mc-stan.org/t/use-of-for-overloaded-distribution-statements/37238
I tried a few solutions. Essentially, when lowering from the AST to MIR, a tilde needs to know:
This information is resolved during typechecking, but currently gets thrown away.
The simplest solution I settled on is 1) adding a separate FnLpmf case to the Fun_kind.suffix type (it always bothered me that FnLpdf lumped these two together, anyway) and then 2) storing this in the Tilde node during typechecking
Submission Checklist
Release notes
Fixed an issue where certain overloads of distributions could lead to the
~statement producing uncompilable C++.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)