Skip to content

Refactor alpha_recover and inverse_hessian_factors to remove update_m… #462

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

Merged
merged 4 commits into from
Apr 19, 2025

Conversation

aphc14
Copy link
Contributor

@aphc14 aphc14 commented Apr 18, 2025

Starting a working draft to modify alpha_recover and inverse_hessian_factors functions.

Following changes by #461, the LBFGS history array, x, g, should be a filtered array using the same conditions previously set by update_mask. Therefore, removing the need for update_mask in alpha_recover and inverse_hessian_factors, and pytensor.scan in inverse_hessian_factors.

@aphc14 aphc14 closed this Apr 19, 2025
@aphc14 aphc14 force-pushed the pf-speedup-hessian-calcs branch from 1cc9962 to f08a40f Compare April 19, 2025 14:04
…ask parameter

- Removed the update_mask variable from alpha_recover and inverse_hessian_factors functions.
- Simplified the logic in alpha_recover by directly computing alpha without filtering updates.
- Changes should offer speed-ups by reducing reliance on scan functions, and perform vectorised operations.
@aphc14 aphc14 reopened this Apr 19, 2025
aphc14 added 3 commits April 20, 2025 00:48
- Corrected the condition for LOW_UPDATE_PCT in LBFGS status handling.
- Removed update_mask references in alpha_recover and inverse_hessian_factors
- Adjusted test cases to reflect changes in status messages and function signatures.
@aphc14 aphc14 marked this pull request as ready for review April 19, 2025 17:16
@aphc14
Copy link
Contributor Author

aphc14 commented Apr 19, 2025

Can we include this in the 0.2.5 release if it passes all checks?

@zaxtax zaxtax requested a review from fonnesbeck April 19, 2025 20:50
Copy link
Member

@fonnesbeck fonnesbeck left a comment

Choose a reason for hiding this comment

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

LGTM

@fonnesbeck fonnesbeck merged commit 4431749 into pymc-devs:main Apr 19, 2025
16 checks passed
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