From fec9ce5ce64d44c98406977f1a9afdc95c8a39c8 Mon Sep 17 00:00:00 2001 From: VsevolodX Date: Wed, 7 Oct 2026 15:15:15 -0700 Subject: [PATCH 1/6] fix(SOF-8067): an element missing from the chemical potentials has a zero shift get_formation_energy_at_chemical_potentials raised KeyError when CHEMICAL_POTENTIALS left out an element of the job, e.g. {"O": -5.346} on V_O in HfO2. Such an element now stays at the reference the job used (delta_mu = 0). Co-Authored-By: Claude Opus 5.5 (1M context) --- .../notebooks_utils/core/entity/property/defect_analysis.py | 5 +---- tests/py/unit/core/entity/test_property_defect_analysis.py | 6 +----- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/py/mat3ra/notebooks_utils/core/entity/property/defect_analysis.py b/src/py/mat3ra/notebooks_utils/core/entity/property/defect_analysis.py index e457e5087..72e753ad9 100644 --- a/src/py/mat3ra/notebooks_utils/core/entity/property/defect_analysis.py +++ b/src/py/mat3ra/notebooks_utils/core/entity/property/defect_analysis.py @@ -141,12 +141,9 @@ def get_formation_energy_at_chemical_potentials(result: DefectJobResult, delta_m E_i is the elemental energy per atom the job used, so the stored value is the one at delta_mu = 0: E_f(delta_mu) = E_f - sum_i dN_i * delta_mu[i]. - - Raises: - KeyError: If `delta_mu` has no value for an element of the job, including one with dN_i = 0. """ return result.formation_energy - sum( - count * delta_mu[element] for element, count in result.delta_n_by_symbol.items() + count * delta_mu.get(element, 0.0) for element, count in result.delta_n_by_symbol.items() ) diff --git a/tests/py/unit/core/entity/test_property_defect_analysis.py b/tests/py/unit/core/entity/test_property_defect_analysis.py index 3ae40a13d..a0e90f643 100644 --- a/tests/py/unit/core/entity/test_property_defect_analysis.py +++ b/tests/py/unit/core/entity/test_property_defect_analysis.py @@ -107,17 +107,13 @@ def test_get_defect_job_result(job, expected): (ZR_HF_RESULT, ZR_HF_DELTA_MU, 0.0134), (V_O_RESULT, V_O_DELTA_MU, 1.010), (V_O_RESULT, ZR_HF_DELTA_MU, 6.356), # Zr is not an element of the job, so its delta_mu is ignored + (ZR_HF_RESULT, V_O_DELTA_MU, 0.3664), # Zr has no delta_mu, so it is 0 ], ) def test_get_formation_energy_at_chemical_potentials(result, delta_mu, expected): assert get_formation_energy_at_chemical_potentials(result, delta_mu) == pytest.approx(expected, abs=1e-4) -def test_get_formation_energy_at_chemical_potentials_raises_on_a_missing_element(): - with pytest.raises(KeyError): - get_formation_energy_at_chemical_potentials(ZR_HF_RESULT, V_O_DELTA_MU) - - @pytest.mark.parametrize( "delta_n_by_symbol, expected", [({"O": -1, "Hf": 0}, "Δμ_O"), ({"Zr": 1, "Hf": -1, "O": 0}, "Δμ_Hf − Δμ_Zr"), ({"O": -2}, "2Δμ_O")], From 4e0e88468e3d863ec905189dd09fac4586559ad4 Mon Sep 17 00:00:00 2001 From: VsevolodX Date: Wed, 7 Oct 2026 15:15:46 -0700 Subject: [PATCH 2/6] fix(SOF-8067): convex hull keeps the GROUP filter when it matches energies by k-grid With SCF_KGRID set, analyze_convex_hull took the total energy of any job on the k-grid, whatever its group. find_job_for_material_with_property takes an optional group regex for the property it checks, and the notebook passes GROUP. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../workflows/analyze_convex_hull.ipynb | 2 +- .../notebooks_utils/core/entity/job/api.py | 8 +++++- tests/py/unit/core/entity/test_job_api.py | 27 ++++++++++++++++--- 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/other/materials_designer/workflows/analyze_convex_hull.ipynb b/other/materials_designer/workflows/analyze_convex_hull.ipynb index 58f4c8cd4..3f5f44169 100644 --- a/other/materials_designer/workflows/analyze_convex_hull.ipynb +++ b/other/materials_designer/workflows/analyze_convex_hull.ipynb @@ -272,7 +272,7 @@ " property_holder = properties_by_exabyte_id.get(exabyte_id)\n", " if SCF_KGRID:\n", " materials_with_exabyte_id = client.materials.list({\"exabyteId\": exabyte_id, \"owner._id\": ACCOUNT_ID})\n", - " jobs = (find_job_for_material_with_property(client, candidate[\"_id\"], \"total_energy\", ACCOUNT_ID, kgrid=SCF_KGRID) for candidate in materials_with_exabyte_id)\n", + " jobs = (find_job_for_material_with_property(client, candidate[\"_id\"], \"total_energy\", ACCOUNT_ID, kgrid=SCF_KGRID, group=GROUP) for candidate in materials_with_exabyte_id)\n", " job = next(filter(None, jobs), None)\n", " property_holder = {\"data\": client.properties.get_for_job(job[\"_id\"], property_name=\"total_energy\")[0]} if job else None\n", " if not property_holder:\n", diff --git a/src/py/mat3ra/notebooks_utils/core/entity/job/api.py b/src/py/mat3ra/notebooks_utils/core/entity/job/api.py index 363c5b2f9..a1c0cf5ff 100644 --- a/src/py/mat3ra/notebooks_utils/core/entity/job/api.py +++ b/src/py/mat3ra/notebooks_utils/core/entity/job/api.py @@ -193,6 +193,7 @@ def find_job_for_material_with_property( owner_id: str, tags: Optional[List[str]] = None, kgrid: Optional[List[int]] = None, + group: Optional[str] = None, ) -> Optional[dict]: """ Finds a finished job on a material that reported the given property, optionally among the jobs @@ -206,6 +207,7 @@ def find_job_for_material_with_property( owner_id (str): Account ID the job must belong to. tags (List[str], optional): Tags the job must all carry. kgrid (List[int], optional): Exact k-grid dimensions the job's `pw_scf` unit ran on; None for no condition. + group (str, optional): Regex the property's group must match, e.g. "qe:dft:gga:pbe"; None for no condition. Returns: dict, optional: The first matching job, or None if none exists. @@ -213,9 +215,13 @@ def find_job_for_material_with_property( query: Dict[str, Any] = {"_material._id": material_id, "owner._id": owner_id, "status": "finished"} if tags: query["tags"] = {"$all": list(tags)} + property_query: Dict[str, Any] = {"data.name": property_name} + if group: + property_query["group"] = {"$regex": group} jobs = api_client.jobs.list({**query, **get_kgrid_query(kgrid)}) return next( - (job for job in jobs if api_client.properties.get_for_job(job["_id"], property_name=property_name)), None + (job for job in jobs if api_client.properties.list(query={"source.info.jobId": job["_id"], **property_query})), + None, ) diff --git a/tests/py/unit/core/entity/test_job_api.py b/tests/py/unit/core/entity/test_job_api.py index d676a45b5..ee724946f 100644 --- a/tests/py/unit/core/entity/test_job_api.py +++ b/tests/py/unit/core/entity/test_job_api.py @@ -149,8 +149,8 @@ def test_find_job_for_material_defaults_to_finished_only(): def test_find_job_for_material_with_property_returns_the_first_job_that_reported_it(): client = MagicMock() client.jobs.list.return_value = [JOB_WITHOUT_PROPERTY, EXISTING_JOB] - client.properties.get_for_job.side_effect = lambda job_id, property_name: ( - [{"name": property_name}] if job_id == EXISTING_JOB["_id"] else [] + client.properties.list.side_effect = lambda query: ( + [{"data": {"name": PROPERTY_NAME}}] if query["source.info.jobId"] == EXISTING_JOB["_id"] else [] ) job = find_job_for_material_with_property( @@ -171,7 +171,7 @@ def test_find_job_for_material_with_property_returns_the_first_job_that_reported def test_find_job_for_material_with_property_returns_none_when_no_job_reported_it(): client = MagicMock() client.jobs.list.return_value = [JOB_WITHOUT_PROPERTY] - client.properties.get_for_job.return_value = [] + client.properties.list.return_value = [] job = find_job_for_material_with_property(client, MATERIAL_INITIAL["_id"], PROPERTY_NAME, OWNER_ID) @@ -202,7 +202,7 @@ def test_get_kgrid_query(kgrid, unit_name, expected_query): def test_find_job_for_material_with_property_matches_the_pw_scf_kgrid(): client = MagicMock() client.jobs.list.return_value = [EXISTING_JOB] - client.properties.get_for_job.return_value = [{"name": PROPERTY_NAME}] + client.properties.list.return_value = [{"data": {"name": PROPERTY_NAME}}] job = find_job_for_material_with_property(client, MATERIAL_INITIAL["_id"], PROPERTY_NAME, OWNER_ID, kgrid=[4, 4, 4]) @@ -210,6 +210,25 @@ def test_find_job_for_material_with_property_matches_the_pw_scf_kgrid(): assert SCF_KGRID_QUERY.items() <= client.jobs.list.call_args.args[0].items() +GROUP = "qe:dft:gga:pbe" +PROPERTY_QUERY: Dict[str, Any] = {"source.info.jobId": EXISTING_JOB["_id"], "data.name": PROPERTY_NAME} + + +@pytest.mark.parametrize( + ("group", "expected_property_query"), + [(None, PROPERTY_QUERY), (GROUP, {**PROPERTY_QUERY, "group": {"$regex": GROUP}})], +) +def test_find_job_for_material_with_property_matches_the_group(group, expected_property_query): + client = MagicMock() + client.jobs.list.return_value = [EXISTING_JOB] + client.properties.list.return_value = [{"data": {"name": PROPERTY_NAME}}] + + job = find_job_for_material_with_property(client, MATERIAL_INITIAL["_id"], PROPERTY_NAME, OWNER_ID, group=group) + + assert job == EXISTING_JOB + client.properties.list.assert_called_once_with(query=expected_property_query) + + def test_find_job_for_material_matches_the_kgrid_of_the_unit(): client = MagicMock() client.jobs.list.return_value = [EXISTING_JOB] From 3e97a70a479d6844dab9f5af6f3387a72a9a18c6 Mon Sep 17 00:00:00 2001 From: VsevolodX Date: Wed, 7 Oct 2026 15:18:57 -0700 Subject: [PATCH 3/6] fix(SOF-8067): convex hull takes a k-grid per formula One SCF_KGRID for every material left the elemental references without an energy: Hf and Zr run on their own converged grids, O2 on another, next to the oxides. SCF_KGRID_BY_FORMULA maps a formula to the k-grid of its total energy job; a formula not listed takes the highest-precision energy (or PRECISION), as before. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../workflows/analyze_convex_hull.ipynb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/other/materials_designer/workflows/analyze_convex_hull.ipynb b/other/materials_designer/workflows/analyze_convex_hull.ipynb index 3f5f44169..76698b721 100644 --- a/other/materials_designer/workflows/analyze_convex_hull.ipynb +++ b/other/materials_designer/workflows/analyze_convex_hull.ipynb @@ -75,7 +75,7 @@ "# Precision value. None = use the best (highest) available. Specific value = use exactly that.\n", "PRECISION = None\n", "\n", - "SCF_KGRID = None # e.g. [4, 4, 4]; None = highest precision\n", + "SCF_KGRID_BY_FORMULA = {} # formula → k-grid of its total energy job; a formula not listed uses PRECISION\n", "\n", "# Organization to select.\n", "ORGANIZATION_NAME = None\n" @@ -225,7 +225,7 @@ "source": [ "## 4. Retrieve total energies\n", "\n", - "For each material, find completed jobs and extract total energy properties. With `SCF_KGRID` set, the energy comes from the material's finished job whose `pw_scf` unit ran on that k-grid; otherwise from the highest-precision property (or exactly `PRECISION`)." + "For each material, find completed jobs and extract total energy properties. For a formula in `SCF_KGRID_BY_FORMULA`, the energy comes from the material's finished job whose `pw_scf` unit ran on that formula's k-grid; otherwise from the highest-precision property (or exactly `PRECISION`)." ] }, { @@ -270,9 +270,10 @@ " seen_exabyte_ids.add(exabyte_id)\n", "\n", " property_holder = properties_by_exabyte_id.get(exabyte_id)\n", - " if SCF_KGRID:\n", + " kgrid = SCF_KGRID_BY_FORMULA.get(material[\"formula\"])\n", + " if kgrid:\n", " materials_with_exabyte_id = client.materials.list({\"exabyteId\": exabyte_id, \"owner._id\": ACCOUNT_ID})\n", - " jobs = (find_job_for_material_with_property(client, candidate[\"_id\"], \"total_energy\", ACCOUNT_ID, kgrid=SCF_KGRID, group=GROUP) for candidate in materials_with_exabyte_id)\n", + " jobs = (find_job_for_material_with_property(client, candidate[\"_id\"], \"total_energy\", ACCOUNT_ID, kgrid=kgrid, group=GROUP) for candidate in materials_with_exabyte_id)\n", " job = next(filter(None, jobs), None)\n", " property_holder = {\"data\": client.properties.get_for_job(job[\"_id\"], property_name=\"total_energy\")[0]} if job else None\n", " if not property_holder:\n", @@ -283,7 +284,7 @@ " number_of_atoms = len(material[\"basis\"][\"coordinates\"])\n", " composition = dict(Counter(element[\"value\"] for element in elements))\n", " energy = property_holder[\"data\"][\"value\"]\n", - " source = f\"job {job['_id']}\" if SCF_KGRID else f\"precision={property_holder.get('precision', {}).get('value', '?')}\"\n", + " source = f\"job {job['_id']}\" if kgrid else f\"precision={property_holder.get('precision', {}).get('value', '?')}\"\n", "\n", " entries_data.append({\"material_id\": material[\"_id\"], \"formula\": material[\"formula\"], \"composition\": composition, \"n_atoms\": number_of_atoms, \"total_energy\": energy})\n", " print(f\"\\u2705 {material['formula']} ({source}): {energy:.4f} eV ({energy / number_of_atoms:.4f} eV/atom)\")\n", From 08f8ecfddb821e3b0ebc99e10420051e1fa2af9a Mon Sep 17 00:00:00 2001 From: VsevolodX Date: Wed, 7 Oct 2026 15:20:18 -0700 Subject: [PATCH 4/6] fix(SOF-8067): relaxed-structure lookup keeps its relaxation unit filter without a k-grid find_relaxed_material filtered by the relaxation unit only through the k-grid query, so with SCF_KGRID = None any finished job with a new final structure counted as the relaxed host. The unit filter now always applies; the default unit is pw_relax, the fixed-cell relaxation the notebooks that call it without a unit run. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../core/entity/material/api.py | 3 ++- tests/py/unit/core/entity/test_material_api.py | 18 +++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/py/mat3ra/notebooks_utils/core/entity/material/api.py b/src/py/mat3ra/notebooks_utils/core/entity/material/api.py index 4848f1a65..926906f0d 100644 --- a/src/py/mat3ra/notebooks_utils/core/entity/material/api.py +++ b/src/py/mat3ra/notebooks_utils/core/entity/material/api.py @@ -99,7 +99,7 @@ def get_final_structure_for_job(api_client: APIClient, job_id: str) -> Material: def find_relaxed_material( - api_client: APIClient, material, owner_id: str, kgrid: Optional[List[int]] = None, unit_name: str = "pw_scf" + api_client: APIClient, material, owner_id: str, kgrid: Optional[List[int]] = None, unit_name: str = "pw_relax" ) -> Optional[Material]: """ Finds a relaxed version of a material: the final structure of a finished job on a material @@ -118,6 +118,7 @@ def find_relaxed_material( """ ids = [m["_id"] for m in api_client.materials.list({"hash": material.hash, "owner._id": owner_id})] query = {"_material._id": {"$in": ids}, "owner._id": owner_id, "status": "finished"} + query["workflow.subworkflows.units.name"] = unit_name for job in api_client.jobs.list({**query, **get_kgrid_query(kgrid, unit_name)}): properties = api_client.properties.get_for_job(job["_id"], PropertyName.non_scalar.final_structure.value) if not properties: diff --git a/tests/py/unit/core/entity/test_material_api.py b/tests/py/unit/core/entity/test_material_api.py index 31e607df9..7769f1cd3 100644 --- a/tests/py/unit/core/entity/test_material_api.py +++ b/tests/py/unit/core/entity/test_material_api.py @@ -248,6 +248,7 @@ def test_get_or_create_materials_set_requires_one_material(): DEFECTIVE_MATERIAL = SimpleNamespace(hash=DEFECTIVE_HASH) SAVED_DEFECTIVE: Dict[str, Any] = {"_id": "m-defective", "name": "B-vacancy h-BN", "hash": DEFECTIVE_HASH} FINISHED_JOB: Dict[str, Any] = {"_id": "job-1", "name": "Fixed-cell Relaxation", "status": "finished"} +RELAX_QUERY = {"owner._id": OWNER_ID, "status": "finished", "workflow.subworkflows.units.name": "pw_relax"} RELAXED_MATERIAL_DOC: Dict[str, Any] = { **Materials.get_by_name_first_match("Silicon"), "name": "B-vacancy h-BN relaxed", @@ -273,9 +274,7 @@ def test_find_relaxed_material_returns_final_structure_from_the_job(): assert relaxed is not None assert relaxed.name == "B-vacancy h-BN relaxed" client.materials.list.assert_called_once_with({"hash": DEFECTIVE_HASH, "owner._id": OWNER_ID}) - client.jobs.list.assert_called_once_with( - {"_material._id": {"$in": [SAVED_DEFECTIVE["_id"]]}, "owner._id": OWNER_ID, "status": "finished"} - ) + client.jobs.list.assert_called_once_with({"_material._id": {"$in": [SAVED_DEFECTIVE["_id"]]}, **RELAX_QUERY}) client.properties.get_for_job.assert_called_once_with(FINISHED_JOB["_id"], "final_structure") client.materials.get.assert_called_once_with("m-relaxed") @@ -339,9 +338,7 @@ def test_find_relaxed_material_checks_every_same_hash_material(): assert relaxed is not None assert relaxed.name == "B-vacancy h-BN relaxed" - client.jobs.list.assert_called_once_with( - {"_material._id": {"$in": ["m-other", "m-defective"]}, "owner._id": OWNER_ID, "status": "finished"} - ) + client.jobs.list.assert_called_once_with({"_material._id": {"$in": ["m-other", "m-defective"]}, **RELAX_QUERY}) def test_find_relaxed_material_skips_a_final_structure_with_the_same_hash(): @@ -360,18 +357,21 @@ def test_find_relaxed_material_skips_a_final_structure_with_the_same_hash(): assert client.properties.get_for_job.call_count == 2 -def test_find_relaxed_material_matches_the_relaxation_kgrid(): +@pytest.mark.parametrize("kgrid", [None, [4, 4, 4]]) +def test_find_relaxed_material_matches_the_relaxation_unit_and_kgrid(kgrid): client = MagicMock() client.materials.list.return_value = [SAVED_DEFECTIVE] client.jobs.list.return_value = [FINISHED_JOB] client.properties.get_for_job.return_value = [{"materialId": "m-relaxed"}] client.materials.get.return_value = RELAXED_MATERIAL_DOC - relaxed = find_relaxed_material(client, DEFECTIVE_MATERIAL, OWNER_ID, kgrid=[4, 4, 4], unit_name="pw_vc-relax") + relaxed = find_relaxed_material(client, DEFECTIVE_MATERIAL, OWNER_ID, kgrid=kgrid, unit_name="pw_vc-relax") assert relaxed is not None assert relaxed.name == "B-vacancy h-BN relaxed" - assert get_kgrid_query([4, 4, 4], "pw_vc-relax").items() <= client.jobs.list.call_args.args[0].items() + query = client.jobs.list.call_args.args[0] + assert query["workflow.subworkflows.units.name"] == "pw_vc-relax" + assert get_kgrid_query(kgrid, "pw_vc-relax").items() <= query.items() @pytest.mark.parametrize( From e9c0579595c4b1a6604d7b1b39ba35141afc6d17 Mon Sep 17 00:00:00 2001 From: VsevolodX Date: Wed, 7 Oct 2026 15:34:13 -0700 Subject: [PATCH 5/6] fix(SOF-8067): docstrings state the always-on unit filter and the zero shift for a missing element Co-Authored-By: Claude Opus 5.5 (1M context) --- src/py/mat3ra/notebooks_utils/core/entity/material/api.py | 5 ++--- .../notebooks_utils/core/entity/property/defect_analysis.py | 2 +- tests/py/unit/core/entity/test_material_api.py | 6 +++++- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/py/mat3ra/notebooks_utils/core/entity/material/api.py b/src/py/mat3ra/notebooks_utils/core/entity/material/api.py index 926906f0d..db392d715 100644 --- a/src/py/mat3ra/notebooks_utils/core/entity/material/api.py +++ b/src/py/mat3ra/notebooks_utils/core/entity/material/api.py @@ -102,9 +102,8 @@ def find_relaxed_material( api_client: APIClient, material, owner_id: str, kgrid: Optional[List[int]] = None, unit_name: str = "pw_relax" ) -> Optional[Material]: """ - Finds a relaxed version of a material: the final structure of a finished job on a material - with the same structural hash, where the geometry has changed, optionally among the jobs whose - `unit_name` unit ran on `kgrid`. + Finds a relaxed version of a material: the final structure of a finished job that ran a `unit_name` unit on a + material with the same structural hash, where the geometry has changed, optionally on `kgrid`. Args: api_client (APIClient): API client instance carrying the authorization context. diff --git a/src/py/mat3ra/notebooks_utils/core/entity/property/defect_analysis.py b/src/py/mat3ra/notebooks_utils/core/entity/property/defect_analysis.py index 72e753ad9..7a2e04636 100644 --- a/src/py/mat3ra/notebooks_utils/core/entity/property/defect_analysis.py +++ b/src/py/mat3ra/notebooks_utils/core/entity/property/defect_analysis.py @@ -140,7 +140,7 @@ def get_formation_energy_at_chemical_potentials(result: DefectJobResult, delta_m Defect formation energy at the chemical potentials mu_i = E_i + delta_mu[i]. E_i is the elemental energy per atom the job used, so the stored value is the one at delta_mu = 0: - E_f(delta_mu) = E_f - sum_i dN_i * delta_mu[i]. + E_f(delta_mu) = E_f - sum_i dN_i * delta_mu[i], where an element missing from `delta_mu` has delta_mu = 0. """ return result.formation_energy - sum( count * delta_mu.get(element, 0.0) for element, count in result.delta_n_by_symbol.items() diff --git a/tests/py/unit/core/entity/test_material_api.py b/tests/py/unit/core/entity/test_material_api.py index 7769f1cd3..54b3edf82 100644 --- a/tests/py/unit/core/entity/test_material_api.py +++ b/tests/py/unit/core/entity/test_material_api.py @@ -248,7 +248,11 @@ def test_get_or_create_materials_set_requires_one_material(): DEFECTIVE_MATERIAL = SimpleNamespace(hash=DEFECTIVE_HASH) SAVED_DEFECTIVE: Dict[str, Any] = {"_id": "m-defective", "name": "B-vacancy h-BN", "hash": DEFECTIVE_HASH} FINISHED_JOB: Dict[str, Any] = {"_id": "job-1", "name": "Fixed-cell Relaxation", "status": "finished"} -RELAX_QUERY = {"owner._id": OWNER_ID, "status": "finished", "workflow.subworkflows.units.name": "pw_relax"} +RELAX_QUERY: Dict[str, Any] = { + "owner._id": OWNER_ID, + "status": "finished", + "workflow.subworkflows.units.name": "pw_relax", +} RELAXED_MATERIAL_DOC: Dict[str, Any] = { **Materials.get_by_name_first_match("Silicon"), "name": "B-vacancy h-BN relaxed", From 3f4af123d0a8b6f134c9ea049d86ed00fb8be5ef Mon Sep 17 00:00:00 2001 From: VsevolodX Date: Wed, 7 Oct 2026 19:24:23 -0700 Subject: [PATCH 6/6] fix(SOF-8067): keep only what the four fixes need in tests and docstrings The group check is one more row of the k-grid test, the relaxation lookup keeps its original k-grid test, and the formation-energy docstring drops the added clause. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../core/entity/property/defect_analysis.py | 2 +- tests/py/unit/core/entity/test_job_api.py | 28 +++++-------------- .../py/unit/core/entity/test_material_api.py | 15 +++------- 3 files changed, 12 insertions(+), 33 deletions(-) diff --git a/src/py/mat3ra/notebooks_utils/core/entity/property/defect_analysis.py b/src/py/mat3ra/notebooks_utils/core/entity/property/defect_analysis.py index 7a2e04636..72e753ad9 100644 --- a/src/py/mat3ra/notebooks_utils/core/entity/property/defect_analysis.py +++ b/src/py/mat3ra/notebooks_utils/core/entity/property/defect_analysis.py @@ -140,7 +140,7 @@ def get_formation_energy_at_chemical_potentials(result: DefectJobResult, delta_m Defect formation energy at the chemical potentials mu_i = E_i + delta_mu[i]. E_i is the elemental energy per atom the job used, so the stored value is the one at delta_mu = 0: - E_f(delta_mu) = E_f - sum_i dN_i * delta_mu[i], where an element missing from `delta_mu` has delta_mu = 0. + E_f(delta_mu) = E_f - sum_i dN_i * delta_mu[i]. """ return result.formation_energy - sum( count * delta_mu.get(element, 0.0) for element, count in result.delta_n_by_symbol.items() diff --git a/tests/py/unit/core/entity/test_job_api.py b/tests/py/unit/core/entity/test_job_api.py index ee724946f..19a7f1622 100644 --- a/tests/py/unit/core/entity/test_job_api.py +++ b/tests/py/unit/core/entity/test_job_api.py @@ -143,6 +143,7 @@ def test_find_job_for_material_defaults_to_finished_only(): PROPERTY_NAME = "total_energy" +GROUP = "qe:dft:gga:pbe" JOB_WITHOUT_PROPERTY: Dict[str, Any] = {"_id": "job-2", "name": "Band Gap", "status": "finished"} @@ -199,34 +200,19 @@ def test_get_kgrid_query(kgrid, unit_name, expected_query): assert get_kgrid_query(kgrid, unit_name) == expected_query -def test_find_job_for_material_with_property_matches_the_pw_scf_kgrid(): +@pytest.mark.parametrize(("group", "expected_property_query"), [(None, {}), (GROUP, {"group": {"$regex": GROUP}})]) +def test_find_job_for_material_with_property_matches_the_pw_scf_kgrid(group, expected_property_query): client = MagicMock() client.jobs.list.return_value = [EXISTING_JOB] client.properties.list.return_value = [{"data": {"name": PROPERTY_NAME}}] - job = find_job_for_material_with_property(client, MATERIAL_INITIAL["_id"], PROPERTY_NAME, OWNER_ID, kgrid=[4, 4, 4]) + job = find_job_for_material_with_property( + client, MATERIAL_INITIAL["_id"], PROPERTY_NAME, OWNER_ID, kgrid=[4, 4, 4], group=group + ) assert job == EXISTING_JOB assert SCF_KGRID_QUERY.items() <= client.jobs.list.call_args.args[0].items() - - -GROUP = "qe:dft:gga:pbe" -PROPERTY_QUERY: Dict[str, Any] = {"source.info.jobId": EXISTING_JOB["_id"], "data.name": PROPERTY_NAME} - - -@pytest.mark.parametrize( - ("group", "expected_property_query"), - [(None, PROPERTY_QUERY), (GROUP, {**PROPERTY_QUERY, "group": {"$regex": GROUP}})], -) -def test_find_job_for_material_with_property_matches_the_group(group, expected_property_query): - client = MagicMock() - client.jobs.list.return_value = [EXISTING_JOB] - client.properties.list.return_value = [{"data": {"name": PROPERTY_NAME}}] - - job = find_job_for_material_with_property(client, MATERIAL_INITIAL["_id"], PROPERTY_NAME, OWNER_ID, group=group) - - assert job == EXISTING_JOB - client.properties.list.assert_called_once_with(query=expected_property_query) + assert expected_property_query.items() <= client.properties.list.call_args.kwargs["query"].items() def test_find_job_for_material_matches_the_kgrid_of_the_unit(): diff --git a/tests/py/unit/core/entity/test_material_api.py b/tests/py/unit/core/entity/test_material_api.py index 54b3edf82..58f975901 100644 --- a/tests/py/unit/core/entity/test_material_api.py +++ b/tests/py/unit/core/entity/test_material_api.py @@ -248,11 +248,7 @@ def test_get_or_create_materials_set_requires_one_material(): DEFECTIVE_MATERIAL = SimpleNamespace(hash=DEFECTIVE_HASH) SAVED_DEFECTIVE: Dict[str, Any] = {"_id": "m-defective", "name": "B-vacancy h-BN", "hash": DEFECTIVE_HASH} FINISHED_JOB: Dict[str, Any] = {"_id": "job-1", "name": "Fixed-cell Relaxation", "status": "finished"} -RELAX_QUERY: Dict[str, Any] = { - "owner._id": OWNER_ID, - "status": "finished", - "workflow.subworkflows.units.name": "pw_relax", -} +RELAX_QUERY = {"owner._id": OWNER_ID, "status": "finished", "workflow.subworkflows.units.name": "pw_relax"} RELAXED_MATERIAL_DOC: Dict[str, Any] = { **Materials.get_by_name_first_match("Silicon"), "name": "B-vacancy h-BN relaxed", @@ -361,21 +357,18 @@ def test_find_relaxed_material_skips_a_final_structure_with_the_same_hash(): assert client.properties.get_for_job.call_count == 2 -@pytest.mark.parametrize("kgrid", [None, [4, 4, 4]]) -def test_find_relaxed_material_matches_the_relaxation_unit_and_kgrid(kgrid): +def test_find_relaxed_material_matches_the_relaxation_kgrid(): client = MagicMock() client.materials.list.return_value = [SAVED_DEFECTIVE] client.jobs.list.return_value = [FINISHED_JOB] client.properties.get_for_job.return_value = [{"materialId": "m-relaxed"}] client.materials.get.return_value = RELAXED_MATERIAL_DOC - relaxed = find_relaxed_material(client, DEFECTIVE_MATERIAL, OWNER_ID, kgrid=kgrid, unit_name="pw_vc-relax") + relaxed = find_relaxed_material(client, DEFECTIVE_MATERIAL, OWNER_ID, kgrid=[4, 4, 4], unit_name="pw_vc-relax") assert relaxed is not None assert relaxed.name == "B-vacancy h-BN relaxed" - query = client.jobs.list.call_args.args[0] - assert query["workflow.subworkflows.units.name"] == "pw_vc-relax" - assert get_kgrid_query(kgrid, "pw_vc-relax").items() <= query.items() + assert get_kgrid_query([4, 4, 4], "pw_vc-relax").items() <= client.jobs.list.call_args.args[0].items() @pytest.mark.parametrize(