Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare Hessian for f_opt too #36

Merged
merged 1 commit into from
Dec 3, 2024
Merged

Conversation

gdalle
Copy link
Contributor

@gdalle gdalle commented Dec 3, 2024

Warning

Merge #37 first

At the moment, the Hessian is prepared for f in the MarginalLogDensity constructor

prep = prepare_hessian(f, hess_adtype, w, Constant(p2))

but computed for f_opt in modal_hessian!

hessian!(mld.f_opt, mld.H, mld.hess_prep, mld.hess_adtype, w, Constant(p2))

This is not allowed by the DifferentiationInterface API, you must prepare with the exact same function you plan to use later. This PR fixes the discrepancy and removes the DI compat upper bound.

@ElOceanografo
Copy link
Owner

Good catch, thanks!

@torfjelde
Copy link

torfjelde commented Dec 5, 2024

Just a heads up: this doesn't seem to have made it's way to a proper "release" yet (currently running into the same issue because I can only install 0.3.4 😕 )

@gdalle
Copy link
Contributor Author

gdalle commented Dec 5, 2024

Actually it has been released, but the general registry is currently lagging (see the conversation on Slack's helpdesk) so you only have access to slightly older versions

@torfjelde
Copy link

Aaah gotcha; cheers:)

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.

3 participants