Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Input File Generation For UCS and PISA SCMSUITE-10150 SO107 #185

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

dormrod
Copy link
Contributor

@dormrod dormrod commented Dec 20, 2024

Description

Efficiency improvements for generating AMS input files for AMSJob. This turned out to be a more complex change than expected due to the fact there are multiple ways of setting up the job:

  • The input settings can either be a Settings object, or an input class from PISA. These settings can contain a System block which may have atoms / system modification keywords (like symmetrise etc.)
  • The molecule can either be one or more Molecule instances or ChemicalSystem

The advantage of PISA/UCS is that these objects can be serialised directly to text, without having to go via a settings object themselves, which can be quite inefficient for large systems.

The complication comes when a system has been supplied on the settings object itself. In this case, for backwards compatibility, the system blocks must be merged. This requires serialisation via settings.

Summary of Behaviour

  • If there is no system block in the input settings and we have PISA, then we can serialise settings straight to text and add the system blocks. If the molecule is a chemical system, this can also be serialised directly to text.
  • If there is a system block and we have PISA, we need to merge them. This requires converting all systems to settings and the PISA to settings, merging them, then overwriting the system block text. This was previously broken as it did not merge/deduplicate information correctly.
  • If we do not have PISA, and there is no system block in the settings, then chemical systems can be serialised straight to text, and molecules as normal.
  • If we do not have PISA, and there is a system block in the settings, then the chemical system must also be serialised to a settings object still

Summary of Improvements

  • In the most common case where there is no system block in the settings, chemical systems will be converted directly to text, avoiding the settings object
  • Existing behaviour has been fixed in a backwards compatible way, so worst case we always serialise to settings objects and combine them
  • Lots of unit tests added for generating these input files through various permutations/combinations of single/multiple molecules/chemical systems with(out) PISA

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.

1 participant