Skip to content

Fix LBFGS iteration conditions and status handling #461

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 1 commit into from
Apr 18, 2025

Conversation

aphc14
Copy link
Contributor

@aphc14 aphc14 commented Apr 18, 2025

hey all,

This PR fixes #445.

During LBFGS iteration, filtering conditions are applied to exclude non-finite values and sharp gradient updates. This PR ensures that when the majority of iterations are excluded, and not enough iterations are stored to run a single PF, that the overall algorithm continues to the next single PF.

Summary:

  • Added entry_condition_met method that combines the logic for determining if LBFGS iterations should be stored.
  • Added LBFGS statuses: LOW_UPDATE_MASK_RATIO to inform user when the majority of LBFGS iters aren't being used.
  • Added LBFGS statuses: INIT_FAILED_LOW_UPDATE_MASK to inform user when LBFGS failed to initialised due to failing to meet the update conditions.
  • Added status messages for LOW_UPDATE_MASK_RATIO and INIT_FAILED_LOW_UPDATE_MASK.
  • Renamed LBFGSStatus.DIVERGED to LBFGSStatus.NON_FINITE.
  • Implemented a test for unstable LBFGS update mask scenarios, adding robustness against rejected iterations.

- Added entry_condition_met method that combines the logic for determining if LBFGS iterations should be stored.
- Added LBFGS statuses: LOW_UPDATE_MASK_RATIO to inform user when majority of LBFGS iters aren't being used.
- Added LBFGS statuses: INIT_FAILED_LOW_UPDATE_MASK to inform user when LBFGS failed to initialised due to failing to meet the update conditions.
- Added status messages for LOW_UPDATE_MASK_RATIO and INIT_FAILED_LOW_UPDATE_MASK.
- Renamed LBFGSStatus.DIVERGED to LBFGSStatus.NON_FINITE.
- Implemented a test for unstable LBFGS update mask scenarios, adding robustness against rejected iterations.
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

@aphc14
Copy link
Contributor Author

aphc14 commented Apr 18, 2025

Demo of the fix, including cases when the number of LBFGS iterations is between 10 and 40 or more, but the stored iterations are 1 or 0.

Example after the fix:
test_lbfgs_missing_gap_handle_nan.ipynb

Whereas previously, all scenarios would've raised an Exception before running PF to completion.

@fonnesbeck fonnesbeck merged commit 73e5e60 into pymc-devs:main Apr 18, 2025
5 checks passed
fonnesbeck pushed a commit to fonnesbeck/pymc-experimental that referenced this pull request Apr 18, 2025
- Added entry_condition_met method that combines the logic for determining if LBFGS iterations should be stored.
- Added LBFGS statuses: LOW_UPDATE_MASK_RATIO to inform user when majority of LBFGS iters aren't being used.
- Added LBFGS statuses: INIT_FAILED_LOW_UPDATE_MASK to inform user when LBFGS failed to initialised due to failing to meet the update conditions.
- Added status messages for LOW_UPDATE_MASK_RATIO and INIT_FAILED_LOW_UPDATE_MASK.
- Renamed LBFGSStatus.DIVERGED to LBFGSStatus.NON_FINITE.
- Implemented a test for unstable LBFGS update mask scenarios, adding robustness against rejected iterations.
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.

pathfinder missing lbfgs_status
2 participants