Skip to content

Replace manual initialization with from_state across integrators and optimizers#420

Merged
orionarcher merged 10 commits intomainfrom
from_state
Jan 27, 2026
Merged

Replace manual initialization with from_state across integrators and optimizers#420
orionarcher merged 10 commits intomainfrom
from_state

Conversation

@orionarcher
Copy link
Collaborator

@orionarcher orionarcher commented Jan 23, 2026

Summary

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.

@orionarcher orionarcher changed the title replace manual initialization with from_state across integrators and optimizers Replace manual initialization with from_state across integrators and optimizers Jan 23, 2026
Copy link
Collaborator

@curtischong curtischong left a comment

Choose a reason for hiding this comment

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

This looks good to me

@curtischong curtischong added the breaking Breaking changes label Jan 24, 2026
@curtischong
Copy link
Collaborator

this PR is breaking since we change the functionality of from_state


# Preserve charge and spin in atoms.info (as integers for FairChem compatibility)
if charge is not None:
atoms.info["charge"] = int(charge[sys_idx].item())
Copy link
Collaborator

@curtischong curtischong Jan 24, 2026

Choose a reason for hiding this comment

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

I think we should change int to float since we use float in other places in the code when talking about charges/spins

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i think we should probably change to int it in the other places then. charge and spin are physically integer values.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should too. it's a bit annoying how charges is a list of float in ASE https://ase-lib.org/ase/atoms.html

Copy link
Member

Choose a reason for hiding this comment

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

spin are physically integer values.

https://en.wikipedia.org/wiki/Fractionalization .... but here yes

Copy link
Collaborator

@thomasloux thomasloux left a comment

Choose a reason for hiding this comment

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

Don't forget to also add that as well to StaticState in runners.py

@orionarcher orionarcher merged commit 0c88f37 into main Jan 27, 2026
67 checks passed
@orionarcher orionarcher deleted the from_state branch January 27, 2026 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaking changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No SimState subclass takes automatically the newly added spin and charge Bug: Spin and charge get lost in optimizers

4 participants