Skip to content

Add Symplectic Integrator#32

Closed
johannes-spies wants to merge 27 commits into
mainfrom
symplectic
Closed

Add Symplectic Integrator#32
johannes-spies wants to merge 27 commits into
mainfrom
symplectic

Conversation

@johannes-spies

Copy link
Copy Markdown

Add a fixed-point iteration based integrator.

@johannes-spies johannes-spies marked this pull request as ready for review February 2, 2026 08:41

@frostedoyster frostedoyster left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The examples/ folder is probably a good idea, but it's too messy at the moment and it needs to be cleaned up. I see some dataset processing code and an ipynb that shouldn't be there.

The symplectic route should also be made available in ASE

Comment thread examples/al/simulation-baseline/baseline.xml Outdated
Comment thread examples/al/simulation-baseline/run.sh Outdated
Comment thread examples/al/.gitignore Outdated
Comment thread src/flashmd/steppers/symplectic.py Outdated
Comment thread src/flashmd/steppers/symplectic.py Outdated
)

# The barostat here needs a simpler splitting than for BZP, something as
# OAbBbBABbAbPO where Bp and Ap are the cell momentum and volume steps

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There is no Bp or Ap in the sequence

@johannes-spies johannes-spies Apr 22, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have to look at the theory for that one. Nevertheless, the functional wrap_* code is just a generalization of what was available in ipi.py all along and the comment just got copied from here

flashmd/src/flashmd/ipi.py

Lines 249 to 250 in 44676ec

# The barostat here needs a simpler splitting than for BZP, something as
# OAbBbBABbAbPO where Bp and Ap are the cell momentum and volume steps

Or did I miss something when copying?

Comment thread src/flashmd/vv.py Outdated
Comment thread src/flashmd/wrappers/nve.py
@johannes-spies

johannes-spies commented Apr 22, 2026

Copy link
Copy Markdown
Author

Thank you very much for your feedback! I tried addressing all your suggestions. Because the PR became very big, here is a short overview of some key design changes.

  • I simplified the example such that it's a single examples/al/al.py file. It's now written in a similar "interactive" Python style, similar to cookbook recipes. tests/test_example_al.py runs a stripped down version (in terms of epochs, simulation steps, and model size) to verify that the example is working, even in CI.
  • To support different AtomisticSteppers, there is a new directory available in src/flashmd/steppers. This is also where the original FlashMDStepper moved to. In addition, there is also a SymplecticStepper. This also has the effect, that some parts (for instance, the thermostat versions in src/flashmd/ase or the wrappers) now have to take an AtomisticStepper and not just a model.
  • To allow different steppers in src/flashmd/ipi.py, I changed the API a bit and introduced the "wrappers" in src/flashmd/wrappers. They contain the functional core bits that lived in the various get_(nve|nvt|npt)_stepper functions before. These functions only allowed changing the model, which was unsuitable for having different kinds of steppers. The code in the wrappers is essentially copy-paste of what the original get_(nve|nvt|npt)_stepper functions did, but a bit more functional.
  • I moved the shared conversion routines to src/flashmd/utils.py. Now, I hope it's easier to convert between various representations of systems: metatomic, ASE, i-PI, and raw position and momenta vectors.

I hope this helps with reviewing this PR!

@johannes-spies

Copy link
Copy Markdown
Author

closed in favor of #43

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