From 4e150f8b1d679f4854a4f2e56f3b72840a0506df Mon Sep 17 00:00:00 2001 From: Zhiyi Wu Date: Mon, 1 Apr 2024 09:47:05 +0100 Subject: [PATCH] Fix incorrect scaling of data_fraction column in forward_backward_convergence (issue #350, PR #355) * fix #350 * The data_fraction column in forward_backward_convergence was scaled by the unit converter in ABFE workflow. This fix ensures that the column values (ranging from 0 to 1) are never accidentally treated as having an energy unit. * add test * update CHANGES --- CHANGES | 4 ++++ src/alchemlyb/tests/test_workflow_ABFE.py | 6 ++++++ src/alchemlyb/workflows/abfe.py | 5 ++++- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index 71fd2912..cabba7b7 100644 --- a/CHANGES +++ b/CHANGES @@ -30,6 +30,10 @@ Enhancements (issue #337, PR #338) - ValueError issued when `df` and `series` for `statistical_inefficiency` doesn't have the same length (issue #337, PR #338) + +Fixes + - data_fraction column in workflow.convergence won't be affected by the + unit conversion (issue #350, PR#319). 22/06/2023 xiki-tempula diff --git a/src/alchemlyb/tests/test_workflow_ABFE.py b/src/alchemlyb/tests/test_workflow_ABFE.py index 0177f292..ed25646d 100644 --- a/src/alchemlyb/tests/test_workflow_ABFE.py +++ b/src/alchemlyb/tests/test_workflow_ABFE.py @@ -368,6 +368,12 @@ def test_ti_convergence(self, workflow, monkeypatch): workflow.check_convergence(10, estimator="TI") assert len(workflow.convergence) == 10 + def test_preserve_unit(self, workflow, monkeypatch): + monkeypatch.setattr(workflow, "convergence", None) + monkeypatch.setattr(workflow, "units", "kcal/mol") + workflow.check_convergence(2, estimator="TI") + assert np.allclose(workflow.convergence["data_fraction"], [0.5, 1.0]) + def test_unprocessed_dhdl(self, workflow, monkeypatch): monkeypatch.setattr(workflow, "dHdl_sample_list", None) monkeypatch.setattr(workflow, "convergence", None) diff --git a/src/alchemlyb/workflows/abfe.py b/src/alchemlyb/workflows/abfe.py index 3ae018c8..751220b4 100644 --- a/src/alchemlyb/workflows/abfe.py +++ b/src/alchemlyb/workflows/abfe.py @@ -801,7 +801,10 @@ def check_convergence( logger.error(msg) raise ValueError(msg) - self.convergence = get_unit_converter(self.units)(convergence) + unit_converted_convergence = get_unit_converter(self.units)(convergence) + # Otherwise the data_fraction column is converted as well. + unit_converted_convergence["data_fraction"] = convergence["data_fraction"] + self.convergence = unit_converted_convergence logger.info(f"Plot convergence analysis to {dF_t} under {self.out}.")