Skip to content

Commit

Permalink
Fix incorrect scaling of data_fraction column in forward_backward_con…
Browse files Browse the repository at this point in the history
…vergence (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
  • Loading branch information
xiki-tempula authored Apr 1, 2024
1 parent 3cf7c25 commit 4e150f8
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 1 deletion.
4 changes: 4 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions src/alchemlyb/tests/test_workflow_ABFE.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion src/alchemlyb/workflows/abfe.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}.")

Expand Down

0 comments on commit 4e150f8

Please sign in to comment.