Skip to content

Add unnormalized log pdf functions - #187

Open
devmotion wants to merge 8 commits into
masterfrom
dw/logupdf
Open

Add unnormalized log pdf functions#187
devmotion wants to merge 8 commits into
masterfrom
dw/logupdf

Conversation

@devmotion

Copy link
Copy Markdown
Member

Fixes #185.

@devmotion
devmotion force-pushed the dw/logupdf branch 2 times, most recently from c740751 to 0a2d5c9 Compare October 20, 2025 04:36
@codecov-commenter

codecov-commenter commented Oct 20, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.72%. Comparing base (eb8e81f) to head (d16071a).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #187      +/-   ##
==========================================
+ Coverage   75.50%   77.72%   +2.21%     
==========================================
  Files          23       23              
  Lines        1135     1239     +104     
==========================================
+ Hits          857      963     +106     
+ Misses        278      276       -2     

☔ View full report in Codecov by Harness.
📢 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.

@devmotion
devmotion force-pushed the dw/logupdf branch 2 times, most recently from 1f8060d to 3e61731 Compare October 22, 2025 18:49
@sethaxen

Copy link
Copy Markdown
Contributor

@devmotion I just noticed you got started on this. Anything I can do to help it along?

@devmotion
devmotion force-pushed the dw/logupdf branch 2 times, most recently from 2dd01ad to 827b221 Compare March 11, 2026 09:50
@devmotion

Copy link
Copy Markdown
Member Author

I had actually forgotten the PR, probably also since nobody reacted to it. I fixed the merge conflicts and applied a few fixes, so tests pass now.

@sethaxen sethaxen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The math/code all looks correct. Just some minor suggestions.

Comment thread src/distrs/gamma.jl Outdated
Comment thread src/distrs/norm.jl Outdated
Comment thread src/distrs/norm.jl Outdated
Comment thread src/distrs/norm.jl Outdated
Comment thread src/distrs/norm.jl Outdated
@devmotion
devmotion marked this pull request as ready for review March 12, 2026 16:02
devmotion and others added 4 commits April 2, 2026 10:24
master (#221) removed the raw combinatorial counts from `signrank` and `wilcox`,
normalising both recurrences in place to avoid `Int` overflow, while this branch
had factored those counts out into `signrank_numsets` and `wilcox_numseqs` so
that the unnormalized log pdfs could be `log(count)`.

Since the normalisation is now folded into the recurrence seed, computing counts
is neither cheaper nor more accurate than computing probabilities, and recovering
`log(count)` by adding the constant back would reintroduce underflow (the sum is
`-Inf` wherever the probability underflows, although `log(count)` is finite
there) and cancellation. So `signranklogupdf` and `wilcoxlogupdf`, like the
`logulikelihood` variants, just return the normalized log pdf -- the same
fallback already used for `hyper`, `nbeta`, `nchisq`, `nfdist` and `ntdist`.
Their tests move to the "fallback" testset accordingly.

Note that `signrank.jl` merged cleanly but incorrectly: `signrank_numsets`
returned master's already-normalised `DP[1]`, so `signrankpdf` divided by `2^n`
a second time and `signranklogupdf` collapsed onto `signranklogpdf`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resolve the conflicts introduced by the TestItems conversion (#209):

- test/runtests.jl: take master's `@run_package_tests`; the "unnormalized"
  entry in the old `tests` array is obsolete now that items are auto-discovered.
- test/chainrules.jl: keep master's `@testitem` wrapper and finite-difference
  margin comment, re-adding the `*logupdf`/`poislogulikelihood` rule tests at
  the new indentation.
- test/unnormalized.jl: convert to `@testitem "Unnormalized optimized"` and
  `@testitem "Unnormalized fallback"`. Without this the file would no longer
  be collected by the runner and none of its tests would execute.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Adding unnormalized logpdf functions

3 participants