Skip to content

Fix remaining BayesianInference benchmark issues - #1552

Open
ArpanC6 wants to merge 2 commits into
SciML:masterfrom
ArpanC6:fix/bayesian-inference-cleanup
Open

Fix remaining BayesianInference benchmark issues#1552
ArpanC6 wants to merge 2 commits into
SciML:masterfrom
ArpanC6:fix/bayesian-inference-cleanup

Conversation

@ArpanC6

@ArpanC6 ArpanC6 commented May 12, 2026

Copy link
Copy Markdown

This PR cleans up and fixes a few lingering issues in the benchmarks/BayesianInference/ folder that were still present after the recent upstream updates.

What Changed

Fixed inconsistencies and small bugs in DiffEqBayesLorenz.jmd, DiffEqBayesFitzHughNagumo.jmd and Project.toml.

Why

After syncing with upstream and reviewing the latest commits, I noticed a few problems:

DiffEqBayesLorenz.jmd was importing DiffEqCallbacks twice.
The Stan sampling call in Lorenz was missing : num_samples => 10_000 which made it inconsistent with LotkaVolterra and FitzHughNagumo.
turing_inference and dynamichmc_inference in Lorenz were using @time instead of @Btime unlike the other benchmarks.
dynamichmc_inference in Lorenz passed solve_kwargs as a named tuple instead of a Dict which did not match the turing_inference call.
DiffEqBayesFitzHughNagumo.jmd was importing DynamicHMC even though dynamichmc_inference is not used there.
All three benchmarks used SciMLBase.FullSpecialize but SciMLBase was not listed in Project.toml.

How

Removed the duplicate DiffEqCallbacks import in Lorenz.
Added : num_samples => 10_000 to the Stan call in Lorenz.
Switched @time to @Btime in Lorenz benchmarks.
Changed solve_kwargs in dynamichmc_inference to a Dict.
Removed the unused DynamicHMC import from FitzHughNagumo.
Added SciMLBase to [deps] and [compat] in Project.toml.

- Remove duplicate DiffEqCallbacks import in Lorenz
- Add num_samples to Lorenz Stan sample_kwargs
- Use @Btime for turing_inference/dynamichmc_inference in Lorenz
- Fix dynamichmc solve_kwargs: named tuple to Dict
- Remove unused DynamicHMC import from FitzHughNagumo
- Add SciMLBase to Project.toml deps and compat
@ArpanC6

ArpanC6 commented May 12, 2026

Copy link
Copy Markdown
Author

Hlw I just opened this PR after syncing with upstream and reviewing the recent commits. Let me know if anything looks off or if any changes are needed.

@ArpanC6

ArpanC6 commented May 12, 2026

Copy link
Copy Markdown
Author
IMG_0223 IMG_0224

While I have been working on these benchmarks I have been thinking about the pseudo likelihood idea you mentioned. Here is my current understanding of why Bayesian methods might actually be better is that the right intuition?

@ChrisRackauckas

Copy link
Copy Markdown
Member

PEM = Prediction Error Method? I don't see how that factors into this.

@ArpanC6

ArpanC6 commented May 13, 2026

Copy link
Copy Markdown
Author

Sorry for the confusion. PEM in my notes was shorthand for p_EM the Euler Maruyama pseudo likelihood not Prediction Error Method.

@ArpanC6

ArpanC6 commented May 14, 2026

Copy link
Copy Markdown
Author

Apologies for the off topic comments. This PR is purely a cleanup no new code no new benchmarks. Happy to rebase or make any changes if needed.

@ArpanC6

ArpanC6 commented Jun 10, 2026

Copy link
Copy Markdown
Author

@ChrisRackauckas Could you please approve the workflows so CI can run? Reduced num_samples to 1_000 to fix the previous timeout. This PR is purely a cleanup no new benchmarks or logic changes.

@ChrisRackauckas

Copy link
Copy Markdown
Member

Approved, but the CI has a lot of backlog.

@ArpanC6

ArpanC6 commented Jun 12, 2026

Copy link
Copy Markdown
Author

The spell check failures are pre existing in other benchmark files and are not related to my changes. My PR only touches DiffEqBayesFitzHughNagumo.jmd DiffEqBayesLorenz.jmd and Project.toml

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.

2 participants