wiederm / endstate_correction

Endstate corrections from MM to QML potential
https://wiederm.github.io/endstate_correction/
MIT License
10 stars 1 forks source link

Update protocol.py #71

Closed wiederm closed 1 year ago

wiederm commented 1 year ago

Critical bug fix: We didn't reverse the order for the bidirectional switching protocol, thus we always switched from reference to target potential independent of at which level the initial samples were created.

wiederm commented 1 year ago

Yes, this is a more than reasonable request. Can you clarify what you mean by 'patch the estimator'?

xiki-tempula commented 1 year ago

So I think we cannot quickly test this function perform_endstate_correction as it would run perform_switching which would run the simulation and it would take some time. For testing purpose, we don't want to spend a lot of time running simulations. So one could replace the function perform_switching with some Patched function that returns some sensible value without actually run the simulation.

So in this case it could be

import pytest
from endstate_correction import perform_endstate_correction

def test_perform_endstate_correction(monkeypatch):
    # Define the mock function for perform_switching
    def mock_perform_switching():
        return "Mocked result"

    # Replace perform_switching with the mock function
    monkeypatch.setattr("endstate_correction.perform_switching", mock_perform_switching)

    # Call perform_endstate_correction
    result = perform_endstate_correction()

    # Check that the result is as expected
    assert result == "Mocked result"
wiederm commented 1 year ago

Oh, that is great, thanks for clarifying! I will add this.

codecov-commenter commented 1 year ago

Codecov Report

Merging #71 (f7c4071) into main (c54a3f8) will increase coverage by 0.12%. The diff coverage is 93.75%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
wiederm commented 1 year ago

I have added a very quick&dirty test for the forw/rev switching protocol based on the sign of the dE's and W's. This is not perfect but would have caught the bug that we have seen here. We need more tests for this, I will open an issue to keep track of that.