Skip to content

AMSWorker Support for ChemicalSystem#376

Draft
dormrod wants to merge 5 commits intofix2026from
DavidOrmrodMorley/amsworker-ucs
Draft

AMSWorker Support for ChemicalSystem#376
dormrod wants to merge 5 commits intofix2026from
DavidOrmrodMorley/amsworker-ucs

Conversation

@dormrod
Copy link
Contributor

@dormrod dormrod commented Mar 12, 2026

Description

This PR adds support for passing scm.base.ChemicalSystem to AMSWorker, alongside PLAMS Molecule, and adds AMSWorkerResults.get_input_system() / get_main_system() in line with AMSResults.

Changes

Main updates:

  • Added ChemicalSystem handling in AMSWorker._prepare_system() via type branching (Molecule vs ChemicalSystem), including serialization of coordinates, charge, lattice, bonds, and atomicInfo
  • Added lazy initialization in AMSWorkerResults for _input_molecule and _input_system, with conversion between representations using same logic as in scm.utils (not used to avoid dependency)
  • Added AMSWorkerResults.get_input_system() and AMSWorkerResults.get_main_system()
  • Existing AMSWorkerResults.get_input_molecule and get_main_molecule now always return copies
  • Added unit tests covering the new ChemicalSystem pathways and parity with existing Molecule behaviour.
  • Integration test to be added in AMSHOME

Points for Comment

  • atomicInfo extraction for ChemicalSystem was a bit tricky to implement - is this the best and most reliable way of doing it?
  • Kept get_main_ase_atoms() based on molecule conversion (toASE(get_main_molecule())), not ChemicalSystem.to_ase_atoms()
  • Probably for after the release to be merged into trunk

@dormrod dormrod marked this pull request as draft March 13, 2026 07:54
@dormrod dormrod changed the title WIP: AMSWorker Support for ChemicalSystem AMSWorker Support for ChemicalSystem Mar 13, 2026
@dormrod dormrod requested a review from robertrueger March 13, 2026 07:59
coords = np.asarray(molecule.coords)
charge = molecule.charge

atomicInfo = []
Copy link
Member

Choose a reason for hiding this comment

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

Check the cs.atom_EOL_string() method. I builds exactly this string of regions + atomic properties that you need here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great thanks, that works perfectly and is much simpler

@dormrod dormrod force-pushed the DavidOrmrodMorley/amsworker-ucs branch from 18a60f1 to 738b2a3 Compare March 13, 2026 09:44
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