Fix pedantic mode false prior counting for distribution arguments

src/analysis_and_optimization/Factor_graph.ml

@@ -143,16 +143,36 @@ let fg_factor_is_prior (var : vexpr) (fac : factor * label)
   (* Check if the data is now unreachable *)
   not (fg_factor_reaches fac data fg')
 
+(** Check if a variable is the 'subject' of a target term factor. For
+    distribution functions (_lpdf/_lpmf and their unnormalized variants), only
+    the first argument is the distribution subject. A factor should only be
+    considered a potential prior for a variable if the variable appears as the
+    subject, not merely as a distribution argument. For non-distribution
+    factors, conservatively return true. *)
+let is_factor_subject (var : vexpr) (factor : factor) : bool =
+  match factor with
+  | TargetTerm
+      { pattern=
+          FunApp
+            ( ( StanLib (_, (FnLpdf _ | FnLpmf _), _)
+              | UserDefined (_, (FnLpdf _ | FnLpmf _)) )
+            , first_arg :: _ )
+      ; _ } ->
+      Set.mem (Set.Poly.map (expr_var_set first_arg) ~f:fst) var
+  | _ -> true
+
 (** Priors of V are neighbors of V which have no connection to any data except
     though V So for graph G and each parameter V: G' = G\V; For each neighbor F:
     Use BFS starting from F in G' and search for any data, if there is none, F
-    is a prior *)
+    is a prior. Additionally, F is only a prior for V if V is the subject (first
+    argument) of a distribution factor, not merely an argument. *)
 let fg_var_priors (var : vexpr) (data : vexpr Set.Poly.t) (fg : factor_graph) :
     (factor * label) Set.Poly.t option =
   match Map.Poly.find fg.var_map var with
   | Some factors ->
       Some
-        (Set.filter factors ~f:(fun fac -> fg_factor_is_prior var fac data fg))
+        (Set.filter factors ~f:(fun ((fac, _) as fl) ->
+             is_factor_subject var fac && fg_factor_is_prior var fl data fg))
   | None -> None
 
 let list_priors ?factor_graph:(fg_opt = None) (mir : Program.Typed.t) :

src/analysis_and_optimization/Pedantic_analysis.ml

@@ -265,8 +265,9 @@ let list_param_dependant_fundefs_cf (mir : Program.Typed.t) :
 let list_non_one_priors (fg : factor_graph) (mir : Program.Typed.t) :
     (string * int * Location_span.t) Set.Poly.t =
   (* Use the factor graph definition of priors, which treats a neighboring
-     factor as a prior for parameter P if it has no connection to the data
-     except through P *)
+     factor as a prior for parameter P if (1) it has no connection to the data
+     except through P, and (2) for distribution factors, P is the distribution
+     subject (first argument) rather than merely a distribution argument *)
   let priors = list_priors ~factor_graph:(Some fg) mir in
   let prior_set =
     Map.Poly.fold priors ~init:Set.Poly.empty
@@ -303,8 +304,8 @@ let compiletime_value_of_expr
 let list_distributions (mir : Program.Typed.t) : dist_info Set.Poly.t =
   let take_dist (expr : Expr.Typed.t) =
     match expr.pattern with
-    | Expr.Pattern.FunApp
-        (StanLib (fname, (FnLpdf true | FnLpmf true), _), arg_exprs) ->
+    | Expr.Pattern.FunApp (StanLib (fname, (FnLpdf _ | FnLpmf _), _), arg_exprs)
+      ->
         let fname = chop_dist_name fname |> Option.value_exn in
         let params = parameter_set mir in
         let data = data_set mir in

test/integration/cli-args/warn-pedantic/pedantic-prior-arg.stan

@@ -0,0 +1,8 @@
+parameters {
+  real x;
+  real sigma;
+}
+model {
+  x ~ normal(0, sigma);
+  sigma ~ exponential(1);
+}

test/integration/cli-args/warn-pedantic/stanc.expected

@@ -12,13 +12,17 @@ Warning in 'distribution-warnings.stan', line 12, column 2 to column 31:
     provided, or the prior(s) depend on data variables. In the later case,
     this may be a false positive.
 Warning in 'distribution-warnings.stan', line 13, column 2 to column 13:
-    The parameter unb_p has 15 priors.
+    The parameter unb_p has no priors. This means either no prior is
+    provided, or the prior(s) depend on data variables. In the later case,
+    this may be a false positive.
 Warning in 'distribution-warnings.stan', line 14, column 2 to column 22:
-    The parameter pos_p has 19 priors.
+    The parameter pos_p has no priors. This means either no prior is
+    provided, or the prior(s) depend on data variables. In the later case,
+    this may be a false positive.
 Warning in 'distribution-warnings.stan', line 15, column 2 to column 32:
-    The parameter unit_p has 2 priors.
-Warning in 'distribution-warnings.stan', line 16, column 2 to column 16:
-    The parameter vec has 2 priors.
+    The parameter unit_p has no priors. This means either no prior is
+    provided, or the prior(s) depend on data variables. In the later case,
+    this may be a false positive.
 Warning in 'distribution-warnings.stan', line 17, column 2 to column 29:
     The parameter pos_vec has no priors. This means either no prior is
     provided, or the prior(s) depend on data variables. In the later case,
@@ -27,8 +31,6 @@ Warning in 'distribution-warnings.stan', line 18, column 2 to column 18:
     The parameter mat has no priors. This means either no prior is provided,
     or the prior(s) depend on data variables. In the later case, this may be
     a false positive.
-Warning in 'distribution-warnings.stan', line 19, column 2 to column 17:
-    The parameter sim has 2 priors.
 Warning in 'distribution-warnings.stan', line 20, column 2 to column 17:
     The parameter ord has no priors. This means either no prior is provided,
     or the prior(s) depend on data variables. In the later case, this may be
@@ -444,7 +446,9 @@ Warning in 'function-param-control-flow.stan', line 15, column 2 to column 26:
 Warning in 'gamma-args.stan', line 2, column 2 to column 18:
     The parameter a has 4 priors.
 Warning in 'gamma-args.stan', line 3, column 2 to column 9:
-    The parameter b has 2 priors.
+    The parameter b has no priors. This means either no prior is provided, or
+    the prior(s) depend on data variables. In the later case, this may be a
+    false positive.
 Warning in 'gamma-args.stan', line 8, column 2 to column 22:
     The parameter a is on the left-hand side of more than one tilde
     statement.
@@ -521,9 +525,9 @@ Warning in 'jacobian_false_alarm.stan', line 7, column 2 to column 20:
 Warning in 'jacobian_false_alarm.stan', line 8, column 2 to column 28:
     The parameter m has 12 priors.
 Warning in 'jacobian_false_alarm.stan', line 9, column 2 to column 9:
-    The parameter y has 9 priors.
+    The parameter y has 7 priors.
 Warning in 'jacobian_false_alarm.stan', line 10, column 2 to column 9:
-    The parameter z has 7 priors.
+    The parameter z has 6 priors.
 [exit 0]
   $ ../../../../../install/default/bin/stanc --warn-pedantic  jacobian_warning1.stan
 Warning in 'jacobian_warning1.stan', line 2, column 2 to column 9:
@@ -633,8 +637,6 @@ Warning in 'missing_priors.stan', line 8, column 2 to column 20:
   $ ../../../../../install/default/bin/stanc --warn-pedantic  multi-tilde.stan
 Warning in 'multi-tilde.stan', line 2, column 2 to column 9:
     The parameter x has 2 priors.
-Warning in 'multi-tilde.stan', line 3, column 2 to column 9:
-    The parameter y has 2 priors.
 Warning in 'multi-tilde.stan', line 6, column 2 to column 19:
     The parameter x is on the left-hand side of more than one tilde
     statement.
@@ -648,10 +650,6 @@ Warning in 'non-one-priors.stan', line 16, column 16 to column 17:
     2), but d was not constrained to be strictly positive.
 [exit 0]
   $ ../../../../../install/default/bin/stanc --warn-pedantic  non-one-priors2.stan
-Warning in 'non-one-priors2.stan', line 6, column 2 to column 9:
-    The parameter a has 3 priors.
-Warning in 'non-one-priors2.stan', line 7, column 2 to column 9:
-    The parameter b has 2 priors.
 Warning in 'non-one-priors2.stan', line 8, column 2 to column 9:
     The parameter c has no priors. This means either no prior is provided, or
     the prior(s) depend on data variables. In the later case, this may be a
@@ -673,16 +671,48 @@ Warning in 'non-one-priors2.stan', line 21, column 2 to column 19:
     statement.
 [exit 0]
   $ ../../../../../install/default/bin/stanc --warn-pedantic  param-depend-control.stan
+Warning in 'param-depend-control.stan', line 2, column 2 to column 9:
+    The parameter a has no priors. This means either no prior is provided, or
+    the prior(s) depend on data variables. In the later case, this may be a
+    false positive.
 Warning in 'param-depend-control.stan', line 8, column 2 to line 12, column 3:
     A control flow statement depends on parameter(s): a.
 Warning in 'param-depend-control.stan', line 13, column 2 to line 15, column 3:
     A control flow statement depends on parameter(s): a.
 Warning in 'param-depend-control.stan', line 16, column 2 to line 18, column 3:
     A control flow statement depends on parameter(s): a.
+[exit 0]
+  $ ../../../../../install/default/bin/stanc --warn-pedantic  pedantic-prior-arg.stan
+Warning in 'pedantic-prior-arg.stan', line 6, column 16 to column 21:
+    A normal distribution is given parameter sigma as a scale parameter
+    (argument 2), but sigma was not constrained to be strictly positive.
+Warning in 'pedantic-prior-arg.stan', line 7, column 2 to column 7:
+    Parameter sigma is given a exponential distribution, which has strictly
+    positive support, but sigma was not constrained to be strictly positive.
 [exit 0]
   $ ../../../../../install/default/bin/stanc --warn-pedantic  unbounded-sigma.stan
 Warning in 'unbounded-sigma.stan', line 2, column 2 to column 9:
     The parameter x has 6 priors.
+Warning in 'unbounded-sigma.stan', line 3, column 2 to column 15:
+    The parameter sigma_a has no priors. This means either no prior is
+    provided, or the prior(s) depend on data variables. In the later case,
+    this may be a false positive.
+Warning in 'unbounded-sigma.stan', line 4, column 2 to column 24:
+    The parameter sigma_b has no priors. This means either no prior is
+    provided, or the prior(s) depend on data variables. In the later case,
+    this may be a false positive.
+Warning in 'unbounded-sigma.stan', line 5, column 2 to column 34:
+    The parameter sigma_c has no priors. This means either no prior is
+    provided, or the prior(s) depend on data variables. In the later case,
+    this may be a false positive.
+Warning in 'unbounded-sigma.stan', line 6, column 2 to column 33:
+    The parameter sigma_d has no priors. This means either no prior is
+    provided, or the prior(s) depend on data variables. In the later case,
+    this may be a false positive.
+Warning in 'unbounded-sigma.stan', line 7, column 2 to column 33:
+    The parameter sigma_e has no priors. This means either no prior is
+    provided, or the prior(s) depend on data variables. In the later case,
+    this may be a false positive.
 Warning in 'unbounded-sigma.stan', line 7, column 2 to column 33:
     Your Stan program has a parameter sigma_e with a lower and upper bound in
     its declaration. These hard constraints are not recommended, for two
@@ -705,9 +735,7 @@ Warning in 'unbounded-sigma.stan', line 12, column 17 to column 24:
     (argument 2), but sigma_c was not constrained to be strictly positive.
 [exit 0]
   $ ../../../../../install/default/bin/stanc --warn-pedantic  uniform.stan
-Warning in 'uniform.stan', line 2, column 2 to column 9:
-    The parameter a has 2 priors.
-Warning in 'uniform.stan', line 8, column 2 to column 20:
+Warning in 'uniform.stan', line 8, column 12 to column 34:
     Parameter a is given a uniform distribution. The uniform distribution is
     not recommended, for two reasons: (a) Except when there are logical or
     physical constraints, it is very unusual for you to be sure that a
@@ -746,8 +774,6 @@ Warning in 'unscaled.stan', line 10, column 20 to column 25:
     https://mc-stan.org/docs/stan-users-guide/efficiency-tuning.html#standardizing-predictors
 [exit 0]
   $ ../../../../../install/default/bin/stanc --warn-pedantic  unused-param.stan
-Warning in 'unused-param.stan', line 3, column 2 to column 9:
-    The parameter b has 2 priors.
 Warning in 'unused-param.stan', line 4, column 2 to column 9:
     The parameter c was declared but was not used in the density calculation.
 Warning in 'unused-param.stan', line 5, column 2 to column 9:

test/integration/cli-args/warn-pedantic/uniform.stan

@@ -5,7 +5,7 @@ parameters {
   real<upper=0> d;
 }
 model {
-  a ~ uniform(0, 1);
+  target += uniform_lpdf(a | 0, 1);
   1 ~ uniform(0, a);
   b ~ uniform(0, 1);
   c ~ uniform(0, 1);

test/unit/Factor_graph.ml

@@ -410,37 +410,7 @@ let%expect_test "Priors complex example" =
               UReal DataOnly))
             (meta ((type_ UReal) (loc <opaque>) (adlevel DataOnly)))))))
         (meta ((type_ UReal) (loc <opaque>) (adlevel AutoDiffable)))))
-      9)
-     ((TargetTerm
-       ((pattern
-         (FunApp (StanLib normal_lpdf (FnLpdf true) AoS)
-          (((pattern (Var e))
-            (meta ((type_ UReal) (loc <opaque>) (adlevel AutoDiffable))))
-           ((pattern (Var a))
-            (meta ((type_ UReal) (loc <opaque>) (adlevel AutoDiffable))))
-           ((pattern
-             (Promotion
-              ((pattern (Lit Int 1))
-               (meta ((type_ UInt) (loc <opaque>) (adlevel DataOnly))))
-              UReal DataOnly))
-            (meta ((type_ UReal) (loc <opaque>) (adlevel DataOnly)))))))
-        (meta ((type_ UReal) (loc <opaque>) (adlevel AutoDiffable)))))
-      14)
-     ((TargetTerm
-       ((pattern
-         (FunApp (StanLib normal_lpdf (FnLpdf true) AoS)
-          (((pattern (Var f))
-            (meta ((type_ UReal) (loc <opaque>) (adlevel AutoDiffable))))
-           ((pattern (Var a))
-            (meta ((type_ UReal) (loc <opaque>) (adlevel AutoDiffable))))
-           ((pattern
-             (Promotion
-              ((pattern (Lit Int 1))
-               (meta ((type_ UInt) (loc <opaque>) (adlevel DataOnly))))
-              UReal DataOnly))
-            (meta ((type_ UReal) (loc <opaque>) (adlevel DataOnly)))))))
-        (meta ((type_ UReal) (loc <opaque>) (adlevel AutoDiffable)))))
-      15)))
+      9)))
    ((begin_loc
      ((filename string) (line_num 7) (col_num 10) (included_from ())))
     (end_loc
@@ -460,22 +430,7 @@ let%expect_test "Priors complex example" =
               UReal DataOnly))
             (meta ((type_ UReal) (loc <opaque>) (adlevel DataOnly)))))))
         (meta ((type_ UReal) (loc <opaque>) (adlevel AutoDiffable)))))
-      10)
-     ((TargetTerm
-       ((pattern
-         (FunApp (StanLib normal_lpdf (FnLpdf true) AoS)
-          (((pattern (Var d))
-            (meta ((type_ UReal) (loc <opaque>) (adlevel AutoDiffable))))
-           ((pattern (Var b))
-            (meta ((type_ UReal) (loc <opaque>) (adlevel AutoDiffable))))
-           ((pattern
-             (Promotion
-              ((pattern (Lit Int 1))
-               (meta ((type_ UInt) (loc <opaque>) (adlevel DataOnly))))
-              UReal DataOnly))
-            (meta ((type_ UReal) (loc <opaque>) (adlevel DataOnly)))))))
-        (meta ((type_ UReal) (loc <opaque>) (adlevel AutoDiffable)))))
-      13)))
+      10)))
    ((begin_loc
      ((filename string) (line_num 8) (col_num 10) (included_from ())))
     (end_loc