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

mhellstr/packmol with current structure #181

Merged
merged 10 commits into from
Dec 16, 2024

Conversation

mhellstr
Copy link
Contributor

@mhellstr mhellstr commented Dec 3, 2024

@dormrod one existing issue with this branch (for example if you run examples/PackMo.py) is that it leaves a plams_workdir folder on disk.

Do you know how to safely delete this if it only contains this kind of packmol job, but definitely not delete it if it contains user jobs (or if user jobs are run after the packmol job)?

See SCMSUITE-10086 and SCMSUITE-7478 for details.

@mhellstr mhellstr changed the title WIP: Mhellstr/packmol with current structure mhellstr/packmol with current structure Dec 13, 2024
@mhellstr mhellstr requested a review from dormrod December 13, 2024 11:25
@mhellstr
Copy link
Contributor Author

Once merged I'll update the regression nightly test output

interfaces/molecule/packmol.py Outdated Show resolved Hide resolved
return my_packed


@requires_optional_package("scm.libbase")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is bold, are we sure we want to limit this in such a way at the moment? None of the other packmol methods have such a restriction, and as libbase can't currently be installed outside amspython, we are limiting its usage to there currently?

trafo = np.linalg.inv(original_ucs.lattice.vectors) @ np.array(target_lattice)
trafo = np.sign(trafo) * np.ceil(np.abs(trafo))
trafo = np.int_(trafo)
supercell.supercell_trafo(trafo)
Copy link
Contributor

@dormrod dormrod Dec 13, 2024

Choose a reason for hiding this comment

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

Also for example the UCS public interface is not yet stable... although I agree it is easier to do this in UCS.

If we don't have UCS available should there always be a fallback to the plams molecule?

Comment on lines +984 to +985
out_ucs = original_ucs.copy()
out_ucs.add_other(distorted)
Copy link
Contributor

Choose a reason for hiding this comment

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

(could be out_ucs = original_ucs + distorted)

@mhellstr mhellstr merged commit dcf916b into trunk Dec 16, 2024
17 checks passed
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