atomate2 Molecule for forcefield

Summary

Discussed in #1123. Following the same strategy as ase tasks, output ForceFieldStructureTaskDocument or ForceFieldMoleculeTaskDocument based on the input type of mol_or_struct.

Checklist

Work-in-progress pull requests are encouraged, but please put [WIP] in the pull request title.

Before a pull request can be merged, the following items must be checked:

[x] Code is in the standard Python style. The easiest way to handle this is to run the following in the correct sequence on your local machine. Start with running ruff and ruff format on your new code. This will automatically reformat your code to PEP8 conventions and fix many linting issues.
[x] Doc strings have been added in the Numpy docstring format. Run ruff on your code.
[x] Type annotations are highly encouraged. Run mypy to type check your code.
[x] Tests have been added for any new functionality or bug fixes.
[x] All linting and tests pass.

Note that the CI system will run all the above checks. But it will be much more efficient if you already fix most errors prior to submitting the PR. It is highly recommended that you use the pre-commit hook provided in the repository. Simply run pre-commit install and a check will be run prior to allowing commits.

Feb 18 '25 16:02 yaoyi92

Dear @JaGeo, can you help take a look at this PR at your convenience?

Feb 18 '25 18:02 yaoyi92

Thank you!

I am tagging @utf and @esoteric-ephemera as well as we have to decide how to handle molecular and structural outputs in general.

Feb 18 '25 19:02 JaGeo

Here is my understanding of how to handle molecular and structural outputs. I saw in atomate2, two approaches exist. In FHI-aims, and cp2k engines, their taskdoc inherents from both StructureMetadata, MoleculeMetadata and the output structure takes the same type of the input structure. While, in ase engine, the implementation is to output StructureTaskDoc or MoleculeTaskDoc based on the input structure. Since, the forcefield inherents mostly from ase, I picked the second approach. The FHI-aims/cp2k approach is cleaner in software engineering. However, StructureMetadata and MoleculeMetadata seems to have different symmetry types. I am not sure whether a conflict will happen is symmetry is used. My understanding is to make them fully compatible with each other would require a rework at the emmet-core level.

Feb 18 '25 19:02 yaoyi92

Hey @yaoyi92, thanks for making this extension to the forcefields! The split that occurs for StructureMetadata and MoleculeMetadata-based task documents in ASE is to ensure there are no collisions between attributes with the same names.

I would avoid merging the ememt-core classes - we're already in the process of reworking the TaskDoc and other schemas in emmet-core to enforce stricter typing

To @JaGeo's point: we have too many disparate schemas for each code, and it would likely be best to have them originate with the same base document models. Maybe we should start an issue for this?

Feb 20 '25 21:02 esoteric-ephemera

@esoteric-ephemera I agree with your points and I also believe we should think about the task docs more broadly.

Feb 20 '25 21:02 JaGeo

I am happy to merge after these changes. I just fear we will have a CI failure now. mp-api and monty need to be updated

Mar 07 '25 17:03 JaGeo

Dear @JaGeo, just check, do we have the CI issue fixed? I think this pull request is read to merge.

Apr 18 '25 15:04 yaoyi92

Thank you. If the tests workflow run through, it will get merged!

Apr 18 '25 15:04 JaGeo

@yaoyi92 still some failures, unfortunately.

Apr 18 '25 15:04 JaGeo

It seems my modification is incompatible with some new commits, let me see what I can do.

Apr 18 '25 16:04 yaoyi92

I fixed the incompatibility issue, how ever it seems there is still some error related to this merge of version bump of mdanalysis #1176.

Apr 18 '25 16:04 yaoyi92

Dear @JaGeo, can you help run the workflow again, I believe the testing can be fixed by this commit 95fd127.

Apr 21 '25 19:04 yaoyi92

@yaoyi92 hey, do you still want to get it merged? Sorry for not following up!

Jun 16 '25 20:06 JaGeo

@yaoyi92 hey, do you still want to get it merged? Sorry for not following up!

Yes, if the CI works fine now. I can try to merge it.

Jun 17 '25 01:06 yaoyi92

Looks like all checks passed. Can you help merge it @JaGeo ?

Jun 22 '25 14:06 yaoyi92

Thanks a lot @yaoyi92 for this! And very sorry to get back to this so late. I've opted to avoid breaking changes as much as possible, and have rearranged the class structures you started slightly.

For backwards compatibility, I've combined your ForceFieldStructureTaskDocument and ForceFieldTaskDocument into the existing ForceFieldTaskDocument. The ForceFieldMoleculeTaskDocument is more or less as you drafted

Dec 03 '25 21:12 esoteric-ephemera