Refactor: unify logjoint/logprior/loglikelihood/returned via internal helper (#1332)

[ArpanC6]

What was the problem?

The problem was that in the code, the functions logjoint, logprior, loglikelihood, and the returned values all followed the same internal pattern. They were each

1. Creating an OnlyAccsVarInfo object with some accumulators.
2. Calling init!! with InitFromParams(..., nothing).
3. Extracting a result from the output.

This pattern was repeated across all four functions resulting in duplicated code. There was no shared way to handle this which made the code harder to maintain and less efficient.

How did you fix it?

To solve this I introduced a single internal helper function called _evaluate_and_extract. This function handles the common steps of

1. Initializing the strategy with InitFromParams using the given parameters.
2. Calling init!! with the model variable information and initialization strategy.
3. Extracting the result using a provided function.

Here’s the implementation

function _evaluate_and_extract(model, params, vi, extract_fn)
init_strategy = InitFromParams(params..., nothing)
return extract_fn(init!!(model, vi, init_strategy, UnlinkAll()))
end

Now each of the original functions (logjoint, logprior, loglikelihood) delegates the work to this helper function. They simply pass their own vi (with the appropriate accumulators) and an extraction function (extract_fn). This reduces code duplication and makes the structure clearer and more reusable.

Note

The function pointwise_logdensities had already been fixed in version v0.41 as mentioned in the issue.

Tests

I ran all 62,988 existing tests locally and everything passed successfully.

This change makes the code more maintainable and easier to understand by reducing redundancy.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.36%. Comparing base (10a3651) to head (9cda745).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1380   +/-   ##
=======================================
  Coverage   82.35%   82.36%           
=======================================
  Files          50       50           
  Lines        3531     3533    +2     
=======================================
+ Hits         2908     2910    +2     
  Misses        623      623           

☔ 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.

[ArpanC6]

All required CI checks are passing. The only failure is Benchmarking / combine results which is a known permission issue for PRs from forks (not related to this change). Could you please review @penelopeysm

[penelopeysm]

Hi @ArpanC6! I should tell you that yesterday was my last day working on Turing. I do feel a bit bad because you just started contributing. I'll review the current PRs you have, but in future, I can't promise to be here to review PRs.

[penelopeysm]

To be completely honest, I think that there is not much benefit in this. The duplication is fairly minimal and it makes the code a bit harder to read because you have to put in extra functions such as x -> getlogprior(last(x)), which make it harder to understand (because you have to mentally 'carry' this function into the helper one).

[penelopeysm]

By putting this function here, you've separated the docstring of logjoint from the implementation of logjoint. The docstring needs to be right above the function, so that the built documentation can correctly find it.

[ArpanC6]

@penelopeysm Thank you so much for reviewing my PRs and for everything you've contributed to Turing. I really appreciate the feedback on this one I'll address the docstring placement and reconsider whether the helper function actually improves readability here.

Congratulations on your next chapter. Is there anyone else on the team I should reach out to for future reviews? I'm enjoying contributing here and would love to keep going.

[penelopeysm]

No problem! I'm sure @yebai will be able to find someone else to review PRs.

[ArpanC6]

@penelopeysm I've addressed your feedback removed the _evaluate_and_extract helper and restored the original implementation for all four functions. Could you please take a look when you get a chance?

[penelopeysm]

Sure, but then it doesn't seem like there are any real changes here.

[ArpanC6]

You're right @penelopeysm since I've reverted the changes there's no net difference from main. I'll close this PR. Thanks for your patience and all your help.