Fix pedantic mode false prior counting for distribution arguments

[ishaan-arora-1]

When a parameter appears as an argument to another parameter's distribution (e.g., sigma in 'x ~ normal(0, sigma)'), pedantic mode was incorrectly counting it as receiving a prior from that factor.

The fix adds an is_factor_subject check to fg_var_priors: for _lpdf/ _lpmf distribution factors, only the first argument (the distribution subject) is considered as receiving a prior. Parameters appearing in other argument positions are distribution arguments, not subjects.

This resolves the false warning 'sigma has 2 priors' when sigma only has one actual prior (sigma ~ exponential(1)) and merely appears as a scale argument in another distribution.

Fixes #746

Submission Checklist

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

Release notes

Replace this text with a short note on what will change if this pull request is merged. This will be included in the release notes.

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)

[WardBrian]

@rybern said in #746 (comment) that this solution (checking for position) "would not be a general solution, since factors in general aren't conditionals"

That being said, just looking at the test output changes, it certainly looks more correct than what we currently have.

[codecov]

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.34%. Comparing base (9236c26) to head (24045b5).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
src/analysis_and_optimization/Factor_graph.ml	87.50%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1603      +/-   ##
==========================================
- Coverage   90.35%   90.34%   -0.02%     
==========================================
  Files          65       65              
  Lines       10028    10035       +7     
==========================================
+ Hits         9061     9066       +5     
- Misses        967      969       +2     

Files with missing lines	Coverage Δ
src/analysis_and_optimization/Pedantic_analysis.ml	95.13% <100.00%> (-0.03%)	⬇️
src/analysis_and_optimization/Factor_graph.ml	71.77% <87.50%> (+0.83%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nhuurre]

"would not be a general solution, since factors in general aren't conditionals" just means that it doesn't know how to handle non-distribution factors, right? This changeset just falls back to previous behaviour for those.

Is the following comment in pedantic analysis still correct?

stanc3/src/analysis_and_optimization/Pedantic_analysis.ml

Lines 267 to 269 in 6014db9

	(* 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 *)

[ishaan-arora-1]

"would not be a general solution, since factors in general aren't conditionals" just means that it doesn't know how to handle non-distribution factors, right? This changeset just falls back to previous behaviour for those.

Is the following comment in pedantic analysis still correct?

stanc3/src/analysis_and_optimization/Pedantic_analysis.ml

Lines 267 to 269 in 6014db9

	(* 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 *)

Good catch — that comment is no longer fully accurate with this change. I'll update it to reflect the additional subject-checking criterion

[ishaan-arora-1]

@nhuurre what about something like :

(* Use the factor graph definition of priors, which treats a neighboring 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 *)"

[ishaan-arora-1]

Is this better now? @jgabry

[jgabry]

Is this better now? @jgabry

I'm not the right person to ask about this kind of change in stanc3. Let's see what @WardBrian and @nhuurre say

[WardBrian]

@nhuurre you might be a better reviewer for this; it looks alright to me but honestly I find a lot of pedantic mode's analysis to be hard to interpret

[nhuurre]

Looks ok to me too but unfortunately I'm not familiar with pedantic analysis code either. Maybe reword the warning that says "This means either no prior is provided, or the prior(s) depend on data variables." since this PR is supposed to reduce "data-dependent prior" problems. Or was this PR was only intended to fix the example from #746 which has no data at all? Note that x ~ normal(0, sigma) was counted as a prior for sigma only if x has no other connection to data. Which makes me think of another example where the factor graph heuristic for priors gets it wrong:

data {
  real a, b;
}
parameters {
  real x, y;
}
model {
  x ~ normal(0,1);
  y ~ normal(x,1); // warning: y has no priors
  a ~ normal(x,1); // unless you delete this line
  b ~ normal(y,1);
}