Skip to content

Fix transverse refinement and transition-state acceptance - #183

Merged
spraharsh merged 11 commits into
mainfrom
transition-state-correctness
Oct 12, 2026
Merged

spraharsh merged 11 commits into
mainfrom
transition-state-correctness

Conversation

@spraharsh

@spraharsh spraharsh commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

Transition-state refinement can step along its excluded reaction mode after the projection changes because transverse L-BFGS retains history from the old plane. It can also accept stationary points with additional downhill directions.

Preserve L-BFGS history when the normalized excluded direction stays the same, including sign flips. Reset it when the plane changes, restoring the caller's configured initial H0. Global L-BFGS reset behavior and optimizer implementations are unchanged.

Before accepting a candidate, require a converged negative primary eigenpair and screen the remaining physical space with two iterative searches. Use an independent private generator that advances between screens; index_seed makes its starts reproducible without consuming the global NumPy stream used by the primary search. Keep the two searches, fixed history and linear workspace. A passing screen is probabilistic and can still miss a negative mode; it is not an exhaustive index-one certificate. No dense Hessian or growing spectral basis is added.

For unit eigenvectors, the old fixed RMS stopping threshold becomes increasingly permissive with dimension. The default secondary-search RMS tolerance is now 1e-5 / sqrt(ndof), giving a dimension-independent gradient-norm threshold. An explicit lowestEigenvectorQuenchParams["tol"] retains its existing RMS meaning. Force and test-comparison tolerances are unchanged. Cold secondary searches retain the eigenvector solver's existing 500-step default; explicit budgets and the warm primary search's default are preserved.

When the screen finds a second negative physical mode, displace downhill along it and continue within the existing outer budget. Unconverged or ambiguous checks return failure. Central gradient differences check the primary curvature and the detected negative pair; all evaluations and returned state remain consistent.

Accounting tests refine a physically perturbed existing LJ18 saddle instead of assuming arbitrary random starts always converge. Transition-state and connection tests explicitly seed both their legacy initialization and private probe starts. The generalized-dimer tests retain their own initialization seed. All success, evaluation-count and iteration assertions remain intact. Deployment notes document arithmetic differences; the solver documentation describes the probabilistic screen.

Validation:

  • Review-fix local non-GUI Python suite: 609 passed, 9 skipped, 2 subtests passed, using existing compiled extensions. The single-threaded run used a guard that stops at 88% system memory use; no local native build was needed.
  • New regressions first reproduced the four-dimensional mode missed by the old fixed guesses, premature positive convergence on a seeded 4,096-dimensional weak-curvature problem, trajectory changes on unchanged/sign-flipped planes, and loss of configured H0. After the fixes all passed. Existing tests cover index-two rejection/recovery, constraints, one dimension, a stiff index-one candidate, exact evaluation counts and independent physical Hessians for stored connection saddles.
  • Seeded gradient-only quadratics retain O(n) workspace. At 262,144 dimensions the old RMS rule missed two negative modes among 16 cases; the scaled rule detected all 16. At 4,096 dimensions with weaker curvature the old rule missed four of eight; the scaled rule detected all eight. Across the 72 sampled models the scaled rule detected every negative mode, without non-convergence. These examples support the stopping correction; they do not establish a per-million failure rate.
  • Earlier macOS traces reproduced the added 100-step verification cap rejecting valid saddles: fixed-coordinate replays needed 103 steps on Apple Silicon and up to 117 on Intel. The added cap was removed in favor of the existing 500-step default.
  • The Intel Python 3.12 trajectory trace recorded 44 searches and 1,064 primary snapshots with no curvature-sign misclassification and exact counts. Four searches correctly returned failure. The older CI failure's exact coordinates were not logged, so this reproduced its abort pattern rather than its exact trajectory.
  • The separate Intel alignment probe showed byte-identical projected vectors producing norms one ULP apart at different memory addresses, propagating into curvature and gradient results. NumPy reports Accelerate; PELE separately loads OpenBLAS. Temporary probes stay outside this PR.
  • Review-fix native CI passed all six jobs on b2ea75df: Linux and macOS source suites each passed 618 tests / 4 skipped / 2 subtests; installed-package suites each passed 609 tests / 13 skipped / 2 subtests; both C++ suites passed.
  • The full portable-wheel matrix passed on b2ea75df: all 12 Linux x86_64, macOS Intel and macOS Apple Silicon wheels for Python 3.11–3.14 each passed 609 tests / 9 skipped / 2 subtests. The source distribution passed, all artifacts uploaded, and PyPI/TestPyPI publishing was skipped.
  • The subsequent f97d6667 commit changes only RELEASING.md; implementation and tests are byte-identical to b2ea75df. Its native CI passed all six jobs. GitHub automatically started another wheel run, which remains in progress.

@codecov-commenter

Copy link
Copy Markdown

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@spraharsh

Copy link
Copy Markdown
Member Author

Adding tracers to make CI run. because there's that strange bug that pops up only on MacOSX and I can only debug via the CI runners

@spraharsh

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-12T00:31:55.382339Z 800e5ec Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 800e5ec5ea

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread pele/transition_states/_transition_state_refinement.py Outdated
Comment thread pele/transition_states/_transverse_walker.py Outdated
Removed detailed explanations about transition-state acceptance checks, secondary searches, and validation processes.
@spraharsh
spraharsh merged commit 3225222 into main Oct 12, 2026
21 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