Convergence tests for protein mutation protocol #95
Conversation
|
As usual, since this is only the tests, CI is expected to fail until we implement the protocol which should pass all the tests. |
|
|
| @@ -192,3 +363,124 @@ def test_asp_to_leu_execute(self, protocol_asp_to_leu_result): | |||
| # the FinishUnit will always be the last to execute | |||
| finishresult = dagresult.protocol_unit_results[-1] | |||
| assert finishresult.name == "result" | |||
|
|
|||
| @pytest.mark.slow | |||
| def test_ala_gly_convergence( | |||
There was a problem hiding this comment.
This test shares a lot of the same code as https://github.com/OpenFreeEnergy/feflow/pull/95/files#diff-82d94bde95c44db8b1006ee5251daf53bea07e1026ed5a559da150c720b090deR263 should it be using the execute_forward_reverse_dag function?
There was a problem hiding this comment.
Yes it definitely should, good catch.
There was a problem hiding this comment.
Actually, on a second thought, it's slightly different because of how the fe estimates and errors are computed for each case. There are ways to decrement the redundant code here but that requires probably better data types/containers to be able to be reused in both cases which I don't think it's worth it for the tests. That is, I think the tests are more readable this way, even if they have some redundant code. But as it's not API it should be fine, imo.
There was a problem hiding this comment.
I just remember the perses tests got really annoying to work with since so many tests were duplicated with just one little change.
I think if you changed execute_forward_reverse_dag to return forward_dagresult, reverse_dagresult instead of the sums then each test would only be the different ways to caclualte errors.
There was a problem hiding this comment.
This should have been addressed in the latest changes. Thanks for the feedback!
for more information, see https://pre-commit.ci
This set of changes includes the tests for the convergence validation.
Solves #79
The idea is that these would be run using the GPU CI workflow, since it's expected to take some hours. I'll be running these in some HPC system to check how long we should expect them to take. Some tweaking of the number of iterations might be needed to optimize the walltimes.