-
Notifications
You must be signed in to change notification settings - Fork 96
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
Fix MPID assignment in electrode workflow #956
Fix MPID assignment in electrode workflow #956
Conversation
So this came out of a need to peg the id of the output document to the "lowest available ID". This is to help prevent id drift over time. Since jobflow has no concept of an mpid but we'd like to recycle the insertion electrode code this seemed like a reasonable compromise. Since we have a situation where many document models are used in systems where a formal MPID is not required, I'm wondering if it makes sense to make the MPID definition a bit broader. |
Thanks, @jmmshn. It might be good in the long run to make emmet's For right now, the flow can't run successfully without passing |
Thanks @esoteric-ephemera |
* fix mpid in electrode * fix mpid in electrode * ensure unique mpid * precommit * change mpid assignment logic to assign ULIDs on the fly as needed * precommit * remove missing test fixture call
Minor bugfix: in
atomate2.common.jobs.electrode
, the utility functionsget_computed_entries
,get_structure_group_doc
, andget_insertion_electrode_doc
assign MPIDs via UUIDs, which leads to pydantic errors when the documents are serialized (an MPID has to have the formmp-<int>
).Thanks to @RobertaPascazio for catching this. Copy @jmmshn to make sure this is still compliant with the flow format.