Skip to content

Fix bugs with constrained GeomOpt & FixSymmetry#552

Open
PascalSalzbrenner wants to merge 7 commits intoTorchSim:mainfrom
PascalSalzbrenner:cell-opt-symmetry
Open

Fix bugs with constrained GeomOpt & FixSymmetry#552
PascalSalzbrenner wants to merge 7 commits intoTorchSim:mainfrom
PascalSalzbrenner:cell-opt-symmetry

Conversation

@PascalSalzbrenner
Copy link
Copy Markdown

Summary

Bugs

I fixed three connected bugs causing high failure rates for constrained high-throughput geometry optimisations.

  • In both FIRE & LBFGS, cell_positions were not updated after the cell was adjusted by constraints, which meant the next iteration started with stale positions. Now they are resynced after the adjustment.
  • In LBFGS, the history vectors mixed two different coordinate frames because prev_positions/forces were stored in the un-adjusted coordinate frame. Now they are stored after the adjustment, consistently with the new positions.
  • FixSymmetry's max_cumulative_strain clamp prevented full geometry optimisation. Speculatively, this was added to prevent strain blow-ups which the other bug fixes seem to have fixed. Removed max_cumulative_strain clamp & reference_cells which were used in the strain clamping. In my testing with heavily distorted structures, they all converge now.

Testing

  • Added constrained optimisation tests for FixSymmetry
  • Add history vectors test for LBFGS optimiser
  • Updated FixSymmetry tests to remove references to max_cumulative_strain clamp & reference_cells

Checklist

Before a pull request can be merged, the following items must be checked:

  • Doc strings have been added in the Google docstring format.
  • Run ruff on your code.
  • Tests have been added for any new functionality or bug fixes.

@CompRhys
Copy link
Copy Markdown
Member

CompRhys commented May 2, 2026

Thanks for the PR! I was out on holiday last week but will take a look in the next few days

@CompRhys
Copy link
Copy Markdown
Member

CompRhys commented May 2, 2026

@janosh Can you provide a repo for the issue motivating #447 as this removes a bunch of what I merged in there as a temp improvement. I confused #447 with #450. Please ignore

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