Error on NaN's in check_model

[penelopeysm]

Closes #887

Open questions:

• Should it warn or error?
• Should it also check for ±Inf?

[github-actions]

Benchmark Report for Commit c215fb2

Computer Information

Julia Version 1.11.4
Commit 8561cc3d68d (2025-03-10 11:36 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 4 × AMD EPYC 7763 64-Core Processor
  WORD_SIZE: 64
  LLVM: libLLVM-16.0.6 (ORCJIT, znver3)
Threads: 1 default, 0 interactive, 1 GC (on 4 virtual cores)

Benchmark Results

|                 Model | Dimension |  AD Backend |      VarInfo Type | Linked | Eval Time / Ref Time | AD Time / Eval Time |
|-----------------------|-----------|-------------|-------------------|--------|----------------------|---------------------|
| Simple assume observe |         1 | forwarddiff |             typed |  false |                  9.5 |                 1.6 |
|           Smorgasbord |       201 | forwarddiff |             typed |  false |                607.9 |                44.4 |
|           Smorgasbord |       201 | forwarddiff | simple_namedtuple |   true |                423.5 |                49.0 |
|           Smorgasbord |       201 | forwarddiff |           untyped |   true |               1212.9 |                28.3 |
|           Smorgasbord |       201 | forwarddiff |       simple_dict |   true |               3739.2 |                21.2 |
|           Smorgasbord |       201 | reversediff |             typed |   true |               1433.4 |                29.9 |
|           Smorgasbord |       201 |    mooncake |             typed |   true |                934.7 |                 5.3 |
|    Loop univariate 1k |      1000 |    mooncake |             typed |   true |               5508.3 |                 4.1 |
|       Multivariate 1k |      1000 |    mooncake |             typed |   true |               1079.1 |                 8.6 |
|   Loop univariate 10k |     10000 |    mooncake |             typed |   true |              60900.7 |                 3.8 |
|      Multivariate 10k |     10000 |    mooncake |             typed |   true |               9158.8 |                 9.5 |
|               Dynamic |        10 |    mooncake |             typed |   true |                128.8 |                13.3 |
|              Submodel |         1 |    mooncake |             typed |   true |                 25.5 |                 8.0 |
|                   LDA |        12 | reversediff |             typed |   true |                478.2 |                 5.0 |

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.92%. Comparing base (019e41b) to head (c215fb2).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #888      +/-   ##
==========================================
+ Coverage   84.87%   84.92%   +0.04%     
==========================================
  Files          34       34              
  Lines        3815     3814       -1     
==========================================
+ Hits         3238     3239       +1     
+ Misses        577      575       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coveralls]

Pull Request Test Coverage Report for Build 14386694475

Details

• 4 of 8 (50.0%) changed or added relevant lines in 1 file are covered.
• 1211 unchanged lines in 26 files lost coverage.
• Overall coverage decreased (-0.5%) to 84.43%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/debug_utils.jl	4	8	50.0%

Files with Coverage Reduction	New Missed Lines	%
src/DynamicPPL.jl	1	90.91%
src/submodel_macro.jl	2	82.35%
src/extract_priors.jl	5	53.57%
src/test_utils/model_interface.jl	5	22.22%
src/test_utils/varinfo.jl	5	78.26%
src/values_as_in_model.jl	6	43.9%
src/logdensityfunction.jl	7	64.44%
src/model_utils.jl	7	7.14%
src/distribution_wrappers.jl	13	9.76%
src/model.jl	13	80.0%

Totals
Change from base Build 14321484767:	-0.5%
Covered Lines:	3221
Relevant Lines:	3815

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 14387051129

Details

• 1 of 3 (33.33%) changed or added relevant lines in 1 file are covered.
• 1211 unchanged lines in 26 files lost coverage.
• Overall coverage increased (+0.05%) to 85.013%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/debug_utils.jl	1	3	33.33%

Files with Coverage Reduction	New Missed Lines	%
src/DynamicPPL.jl	1	90.91%
src/submodel_macro.jl	2	82.35%
src/extract_priors.jl	5	53.57%
src/test_utils/model_interface.jl	5	22.22%
src/test_utils/varinfo.jl	5	78.26%
src/values_as_in_model.jl	6	43.9%
src/logdensityfunction.jl	7	64.44%
src/model_utils.jl	7	7.14%
src/distribution_wrappers.jl	13	9.76%
src/model.jl	13	80.0%

Totals
Change from base Build 14321484767:	0.05%
Covered Lines:	3239
Relevant Lines:	3810

---

💛 - Coveralls

[sunxd3]

Looks good

[penelopeysm]

@sunxd3 do you have an opinion on checking for Inf too? (I edited this in at the top but either it was too late or the bot comments distracted from it 😂)

[sunxd3]

I think checking Inf is a good idea. Also to make it more useful, is there an option where the checking function will do multi-run? (Saying this because Inf can be less deterministic than the other two.)

[yebai]

Should it warn or error?

This should be an error.

Should it also check for ±Inf?

This depends; certain distributions have ±Inf in support. It is better to check whether ±Inf is in support; if not, an informational message can be given.

[penelopeysm]

Ok! I think let's leave Infs for another time/PR. I am a bit wary of edge cases - NaN is more straightforward because you probably don't want any of them anywhere, so any(isnan, left) suffices; but if Inf is sometimes ok and sometimes not (which makes sense to me) then it gets a bit finicky, especially if the rhs has something like arraydist or product_distribution.

[penelopeysm]

Inf is now a new issue, #889, so someone can pick it up if they feel like it. But at least having the NaN's checked will fix the immediate request in TuringLang/Turing.jl#2515.