-
Notifications
You must be signed in to change notification settings - Fork 219
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
Mode estimation's support of Pathfinder integration #2268
Comments
So like |
We should probably wrap this in a function with a human-readable name and docstring though. |
Ah, so it's that easy now, awesome! In the past it required much more code. https://github.com/mlcolab/Pathfinder.jl/blob/f1a12e11ddbcd4557a9b95ecb9218be49aa2b18c/ext/PathfinderTuringExt.jl#L23-L102 is based on some functions you had sent me in the past.
What specifically has been removed? Constrained optimization? For Pathfinder we only do optimization in unconstrained space so we wouldn't need or want constrained optimization.
This would be very helpful! |
Aye, very much agree. The current implementation was done in this way because it needed to replace some hacky stuff we did before. But at this point I very much agree that it would be worth moving to a separate function exposed, maybe in DynamicPPL. Could do call it something like |
Closed by mlcolab/Pathfinder.jl#189. Thanks @sethaxen! |
The recent overhaul of our mode estimation interface breaks things for Pathfinder. @sethaxen raised this on Slack, I'll paste here what he said:
I think the answer to all the three questions is "we could probably add that". In particular
link
on the user-provided initial guess though, which I think isn't what you want. We could look into changing that.ModeResult
stores theOptimLogDensity
as the fieldf
.OptimLogDensity
itself can compute the gradients when called with the right arguments, but internally we don't actually use that feature any more, but rather rely on Optimization.jl's handling of AD. (The Optim.jl extension does use it, which is why it's still there.)In general, the old interface claimed to be able to do both constrained (box bounds) and unconstrained optimisation independently of whether the model was transformed into unconstrained space before optimisation. It didn't actually handle all cases properly though, and thus in the new version this ability has been removed, to not expose broken code paths to users. In the future we would like to implement this properly and enable it again, which I think would help with Pathfinder's integration, but I'm not sure when we'll get to that.
Given that you need access to the internals of the optimisation in quite a different way than what
estimate_mode
assumes, I wonder if the easiest solution would be bypassestimate_mode
completely. This way we wouldn't have to think carefully about how to both support Pathfinder's needs and maintain an interface friendly to non-expert users. I could help you out by making a version of something like the oldoptim_problem
/optim_function
interface, but with only the features Pathfinder was calling, which I think could end up being quite minimal. How Pathfinder calls the old interface seems to be quite neatly encapsulated in two short functions, so seeing what the relevant features are shouldn't be too hard.The text was updated successfully, but these errors were encountered: