From 14727b231d713f23f1e2b15d6e0002b00c3004bf Mon Sep 17 00:00:00 2001 From: esoteric-ephemera Date: Thu, 15 Aug 2024 09:33:14 -0700 Subject: [PATCH 1/7] fix mpid in electrode --- src/atomate2/common/jobs/electrode.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/atomate2/common/jobs/electrode.py b/src/atomate2/common/jobs/electrode.py index 1d3786c077..fe431a37a3 100644 --- a/src/atomate2/common/jobs/electrode.py +++ b/src/atomate2/common/jobs/electrode.py @@ -154,13 +154,13 @@ def get_computed_entries( return multi # keep the [1] for now, if jobflow supports NamedTuple, we can do this much cleaner s_ = RelaxJobSummary._make(single) - s_.entry.entry_id = s_.uuid + s_.entry.entry_id = "mp-0000000" ent = ComputedStructureEntry( structure=s_.structure, energy=s_.entry.energy, parameters=s_.entry.parameters, data=s_.entry.data, - entry_id=s_.uuid, + entry_id="mp-0000000", ) return [*multi, ent] @@ -171,7 +171,9 @@ def get_structure_group_doc( ) -> Response: """Take in `ComputedEntry` and return a `StructureGroupDoc`.""" for ient in computed_entries: - ient.data["material_id"] = ient.entry_id + if not (entry_id := str(entry_id)).startswith("mp-"): + entry_id = "mp-0000000" + ient.data["material_id"] = entry_id return StructureGroupDoc.from_grouped_entries( computed_entries, ignored_specie=ignored_species ) @@ -183,7 +185,9 @@ def get_insertion_electrode_doc( ) -> Response: """Return a `InsertionElectrodeDoc`.""" for ient in computed_entries: - ient.data["material_id"] = ient.entry_id + if not (entry_id := str(entry_id)).startswith("mp-"): + entry_id = "mp-0000000" + ient.data["material_id"] = entry_id return InsertionElectrodeDoc.from_entries( computed_entries, working_ion_entry, battery_id=None ) From 5c20ddedeadf70d28359cb22d28d1867d64d7df9 Mon Sep 17 00:00:00 2001 From: esoteric-ephemera Date: Thu, 15 Aug 2024 09:34:40 -0700 Subject: [PATCH 2/7] fix mpid in electrode --- src/atomate2/common/jobs/electrode.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/atomate2/common/jobs/electrode.py b/src/atomate2/common/jobs/electrode.py index fe431a37a3..932ed205dc 100644 --- a/src/atomate2/common/jobs/electrode.py +++ b/src/atomate2/common/jobs/electrode.py @@ -171,7 +171,7 @@ def get_structure_group_doc( ) -> Response: """Take in `ComputedEntry` and return a `StructureGroupDoc`.""" for ient in computed_entries: - if not (entry_id := str(entry_id)).startswith("mp-"): + if not (entry_id := str(ient.entry_id)).startswith("mp-"): entry_id = "mp-0000000" ient.data["material_id"] = entry_id return StructureGroupDoc.from_grouped_entries( @@ -185,7 +185,7 @@ def get_insertion_electrode_doc( ) -> Response: """Return a `InsertionElectrodeDoc`.""" for ient in computed_entries: - if not (entry_id := str(entry_id)).startswith("mp-"): + if not (entry_id := str(ient.entry_id)).startswith("mp-"): entry_id = "mp-0000000" ient.data["material_id"] = entry_id return InsertionElectrodeDoc.from_entries( From 905c80350e6b60cb3bd0a8bbb843b90500b1af53 Mon Sep 17 00:00:00 2001 From: esoteric-ephemera Date: Thu, 15 Aug 2024 11:26:50 -0700 Subject: [PATCH 3/7] ensure unique mpid --- src/atomate2/common/jobs/electrode.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/atomate2/common/jobs/electrode.py b/src/atomate2/common/jobs/electrode.py index 932ed205dc..e9d867f903 100644 --- a/src/atomate2/common/jobs/electrode.py +++ b/src/atomate2/common/jobs/electrode.py @@ -154,13 +154,14 @@ def get_computed_entries( return multi # keep the [1] for now, if jobflow supports NamedTuple, we can do this much cleaner s_ = RelaxJobSummary._make(single) - s_.entry.entry_id = "mp-0000000" + temp_mpid = f"mp-{len(multi) + 1}" + s_.entry.entry_id = temp_mpid ent = ComputedStructureEntry( structure=s_.structure, energy=s_.entry.energy, parameters=s_.entry.parameters, data=s_.entry.data, - entry_id="mp-0000000", + entry_id=temp_mpid, ) return [*multi, ent] @@ -170,9 +171,9 @@ def get_structure_group_doc( computed_entries: list[ComputedEntry], ignored_species: str ) -> Response: """Take in `ComputedEntry` and return a `StructureGroupDoc`.""" - for ient in computed_entries: + for idx, ient in enumerate(computed_entries): if not (entry_id := str(ient.entry_id)).startswith("mp-"): - entry_id = "mp-0000000" + entry_id = f"mp-{idx+1}" ient.data["material_id"] = entry_id return StructureGroupDoc.from_grouped_entries( computed_entries, ignored_specie=ignored_species @@ -184,9 +185,9 @@ def get_insertion_electrode_doc( computed_entries: ComputedEntry, working_ion_entry: ComputedEntry ) -> Response: """Return a `InsertionElectrodeDoc`.""" - for ient in computed_entries: + for idx, ient in enumerate(computed_entries): if not (entry_id := str(ient.entry_id)).startswith("mp-"): - entry_id = "mp-0000000" + entry_id =f"mp-{idx+1}" ient.data["material_id"] = entry_id return InsertionElectrodeDoc.from_entries( computed_entries, working_ion_entry, battery_id=None From 2c319d189136156badf0ab7f79105069d1f57104 Mon Sep 17 00:00:00 2001 From: esoteric-ephemera Date: Thu, 15 Aug 2024 11:27:09 -0700 Subject: [PATCH 4/7] precommit --- src/atomate2/common/jobs/electrode.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/atomate2/common/jobs/electrode.py b/src/atomate2/common/jobs/electrode.py index e9d867f903..6c020b8008 100644 --- a/src/atomate2/common/jobs/electrode.py +++ b/src/atomate2/common/jobs/electrode.py @@ -187,7 +187,7 @@ def get_insertion_electrode_doc( """Return a `InsertionElectrodeDoc`.""" for idx, ient in enumerate(computed_entries): if not (entry_id := str(ient.entry_id)).startswith("mp-"): - entry_id =f"mp-{idx+1}" + entry_id = f"mp-{idx+1}" ient.data["material_id"] = entry_id return InsertionElectrodeDoc.from_entries( computed_entries, working_ion_entry, battery_id=None From dbaa673a76359f0b0e018c2b660cbfccd4287ab3 Mon Sep 17 00:00:00 2001 From: esoteric-ephemera Date: Mon, 19 Aug 2024 12:04:55 -0700 Subject: [PATCH 5/7] change mpid assignment logic to assign ULIDs on the fly as needed --- src/atomate2/common/jobs/electrode.py | 29 +++++++++++++++++---------- tests/vasp/flows/test_electrode.py | 19 ------------------ 2 files changed, 18 insertions(+), 30 deletions(-) diff --git a/src/atomate2/common/jobs/electrode.py b/src/atomate2/common/jobs/electrode.py index 6c020b8008..a4540166b5 100644 --- a/src/atomate2/common/jobs/electrode.py +++ b/src/atomate2/common/jobs/electrode.py @@ -3,10 +3,12 @@ from __future__ import annotations import logging +from ulid import ULID from typing import TYPE_CHECKING, Callable, NamedTuple from emmet.core.electrode import InsertionElectrodeDoc from emmet.core.structure_group import StructureGroupDoc +from emmet.core.mpid import MPID from jobflow import Flow, Maker, Response, job from pymatgen.analysis.defects.generators import ChargeInterstitialGenerator from pymatgen.entries.computed_entries import ComputedStructureEntry @@ -154,14 +156,23 @@ def get_computed_entries( return multi # keep the [1] for now, if jobflow supports NamedTuple, we can do this much cleaner s_ = RelaxJobSummary._make(single) - temp_mpid = f"mp-{len(multi) + 1}" - s_.entry.entry_id = temp_mpid + + # Ensure that the entry_id is an acceptable MPID + try: + entry_id = MPID(s_.uuid) + except ValueError: + entry_id = ULID() + s_.entry.entry_id = str(entry_id) + + # Store UUID for provenance + s_.entry.data["UUID"] = s_.uuid + ent = ComputedStructureEntry( structure=s_.structure, energy=s_.entry.energy, parameters=s_.entry.parameters, data=s_.entry.data, - entry_id=temp_mpid, + entry_id=s_.entry.entry_id, ) return [*multi, ent] @@ -171,10 +182,8 @@ def get_structure_group_doc( computed_entries: list[ComputedEntry], ignored_species: str ) -> Response: """Take in `ComputedEntry` and return a `StructureGroupDoc`.""" - for idx, ient in enumerate(computed_entries): - if not (entry_id := str(ient.entry_id)).startswith("mp-"): - entry_id = f"mp-{idx+1}" - ient.data["material_id"] = entry_id + for ient in computed_entries: + ient.data["material_id"] = ient.entry_id return StructureGroupDoc.from_grouped_entries( computed_entries, ignored_specie=ignored_species ) @@ -185,10 +194,8 @@ def get_insertion_electrode_doc( computed_entries: ComputedEntry, working_ion_entry: ComputedEntry ) -> Response: """Return a `InsertionElectrodeDoc`.""" - for idx, ient in enumerate(computed_entries): - if not (entry_id := str(ient.entry_id)).startswith("mp-"): - entry_id = f"mp-{idx+1}" - ient.data["material_id"] = entry_id + for ient in computed_entries: + ient.data["material_id"] = ient.entry_id return InsertionElectrodeDoc.from_entries( computed_entries, working_ion_entry, battery_id=None ) diff --git a/tests/vasp/flows/test_electrode.py b/tests/vasp/flows/test_electrode.py index 0ce47c579b..5158266251 100644 --- a/tests/vasp/flows/test_electrode.py +++ b/tests/vasp/flows/test_electrode.py @@ -1,24 +1,5 @@ from __future__ import annotations -from unittest import mock - -import pytest -from jobflow.settings import JobflowSettings - - -@pytest.fixture() -def mock_jobflow_settings(memory_jobstore): - """Set the UID_TYPE to "ulid" to make sure the documents can be sorted. - - See: https://github.com/materialsproject/jobflow/issues/519#issuecomment-1906850096 - """ - - settings = JobflowSettings(JOB_STORE=memory_jobstore, UID_TYPE="ulid") - - with mock.patch("jobflow.SETTINGS", settings): - yield - - def test_electrode_makers(mock_vasp, clean_dir, test_dir, mock_jobflow_settings): from emmet.core.electrode import InsertionElectrodeDoc from jobflow import OutputReference, run_locally From 98ac77027bf72dc17b0557a7ee9dd6d9cd00ecd4 Mon Sep 17 00:00:00 2001 From: esoteric-ephemera Date: Mon, 19 Aug 2024 12:05:54 -0700 Subject: [PATCH 6/7] precommit --- src/atomate2/common/jobs/electrode.py | 4 ++-- tests/vasp/flows/test_electrode.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/atomate2/common/jobs/electrode.py b/src/atomate2/common/jobs/electrode.py index a4540166b5..4ef3a29a02 100644 --- a/src/atomate2/common/jobs/electrode.py +++ b/src/atomate2/common/jobs/electrode.py @@ -3,15 +3,15 @@ from __future__ import annotations import logging -from ulid import ULID from typing import TYPE_CHECKING, Callable, NamedTuple from emmet.core.electrode import InsertionElectrodeDoc -from emmet.core.structure_group import StructureGroupDoc from emmet.core.mpid import MPID +from emmet.core.structure_group import StructureGroupDoc from jobflow import Flow, Maker, Response, job from pymatgen.analysis.defects.generators import ChargeInterstitialGenerator from pymatgen.entries.computed_entries import ComputedStructureEntry +from ulid import ULID if TYPE_CHECKING: from pathlib import Path diff --git a/tests/vasp/flows/test_electrode.py b/tests/vasp/flows/test_electrode.py index 5158266251..315fc0567b 100644 --- a/tests/vasp/flows/test_electrode.py +++ b/tests/vasp/flows/test_electrode.py @@ -1,5 +1,6 @@ from __future__ import annotations + def test_electrode_makers(mock_vasp, clean_dir, test_dir, mock_jobflow_settings): from emmet.core.electrode import InsertionElectrodeDoc from jobflow import OutputReference, run_locally From acce6341881fe257ae3921d0176ab6cd1aa5695c Mon Sep 17 00:00:00 2001 From: esoteric-ephemera Date: Mon, 19 Aug 2024 12:07:26 -0700 Subject: [PATCH 7/7] remove missing test fixture call --- tests/vasp/flows/test_electrode.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/vasp/flows/test_electrode.py b/tests/vasp/flows/test_electrode.py index 315fc0567b..79ab6738b2 100644 --- a/tests/vasp/flows/test_electrode.py +++ b/tests/vasp/flows/test_electrode.py @@ -1,7 +1,7 @@ from __future__ import annotations -def test_electrode_makers(mock_vasp, clean_dir, test_dir, mock_jobflow_settings): +def test_electrode_makers(mock_vasp, clean_dir, test_dir): from emmet.core.electrode import InsertionElectrodeDoc from jobflow import OutputReference, run_locally from monty.serialization import loadfn @@ -14,7 +14,6 @@ def test_electrode_makers(mock_vasp, clean_dir, test_dir, mock_jobflow_settings) update_user_incar_settings, update_user_kpoints_settings, ) - # mock the default setting # mapping from job name to directory containing test files ref_paths = {