search

uw-ipd / tmol

TMol
Apache License 2.0
30 stars 3 forks source link

Implement disulfide score term #254

Closed jflat06 closed 1 year ago

jflat06 commented 1 year ago

Introduces the disulfide bond score term.

The EnergyTerm itself down into the C++ code should be complete.

Limitations:

The loading of the actual disulfides from the test fixtures, all the way up to the annotation of the blocks may require further work in the future, depending on how we want to handle this.

Currently the way of loading and annotating the disulfides in the block types allows for only 1 disulfide per block type. If we develop a system for describing multiple disulfide conenctions per block type, only the annotation step will need to change. Everything else in the EnergyTerm should work fine with multiple disulfide bonds per block.

jflat06 commented 1 year ago

Unlike omega, this is an entirely new score term. Is there any additional work that needs to be done to hook this into everything? I did a grep for omega and it seemed like all other code using omega was using the old omega.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 99.60% and project coverage change: +0.05 :tada:

Comparison is base (add620a) 95.07% compared to head (2d128a7) 95.13%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #254 +/- ## ========================================== + Coverage 95.07% 95.13% +0.05% ========================================== Files 320 327 +7 Lines 20040 20290 +250 ========================================== + Hits 19054 19303 +249 - Misses 986 987 +1 ``` | Flag | Coverage Δ | | |---|---|---| | _shrug_Testing_CPU | `89.68% <99.60%> (+0.12%)` | :arrow_up: | | _shrug_Testing_CPU_w_o_jit | `91.62% <99.60%> (+0.09%)` | :arrow_up: | | _shrug_Testing_CUDA | `92.31% <99.60%> (+0.09%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uw-ipd#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://app.codecov.io/gh/uw-ipd/tmol/pull/254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uw-ipd) | Coverage Δ | | |---|---|---| | [tmol/tests/conftest.py](https://app.codecov.io/gh/uw-ipd/tmol/pull/254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uw-ipd#diff-dG1vbC90ZXN0cy9jb25mdGVzdC5weQ==) | `55.55% <ø> (ø)` | | | [tmol/score/disulfide/disulfide\_energy\_term.py](https://app.codecov.io/gh/uw-ipd/tmol/pull/254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uw-ipd#diff-dG1vbC9zY29yZS9kaXN1bGZpZGUvZGlzdWxmaWRlX2VuZXJneV90ZXJtLnB5) | `97.67% <97.67%> (ø)` | | | [tmol/chemical/restypes.py](https://app.codecov.io/gh/uw-ipd/tmol/pull/254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uw-ipd#diff-dG1vbC9jaGVtaWNhbC9yZXN0eXBlcy5weQ==) | `95.04% <100.00%> (+0.33%)` | :arrow_up: | | [tmol/database/scoring/\_\_init\_\_.py](https://app.codecov.io/gh/uw-ipd/tmol/pull/254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uw-ipd#diff-dG1vbC9kYXRhYmFzZS9zY29yaW5nL19faW5pdF9fLnB5) | `100.00% <100.00%> (ø)` | | | [tmol/database/scoring/disulfide.py](https://app.codecov.io/gh/uw-ipd/tmol/pull/254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uw-ipd#diff-dG1vbC9kYXRhYmFzZS9zY29yaW5nL2Rpc3VsZmlkZS5weQ==) | `100.00% <100.00%> (ø)` | | | [tmol/pose/pose\_stack\_builder.py](https://app.codecov.io/gh/uw-ipd/tmol/pull/254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uw-ipd#diff-dG1vbC9wb3NlL3Bvc2Vfc3RhY2tfYnVpbGRlci5weQ==) | `96.78% <100.00%> (+0.01%)` | :arrow_up: | | [...mol/score/disulfide/disulfide\_whole\_pose\_module.py](https://app.codecov.io/gh/uw-ipd/tmol/pull/254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uw-ipd#diff-dG1vbC9zY29yZS9kaXN1bGZpZGUvZGlzdWxmaWRlX3dob2xlX3Bvc2VfbW9kdWxlLnB5) | `100.00% <100.00%> (ø)` | | | [tmol/score/disulfide/params.py](https://app.codecov.io/gh/uw-ipd/tmol/pull/254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uw-ipd#diff-dG1vbC9zY29yZS9kaXN1bGZpZGUvcGFyYW1zLnB5) | `100.00% <100.00%> (ø)` | | | [tmol/score/disulfide/potentials/compiled.py](https://app.codecov.io/gh/uw-ipd/tmol/pull/254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uw-ipd#diff-dG1vbC9zY29yZS9kaXN1bGZpZGUvcG90ZW50aWFscy9jb21waWxlZC5weQ==) | `100.00% <100.00%> (ø)` | | | [tmol/score/score\_types.py](https://app.codecov.io/gh/uw-ipd/tmol/pull/254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uw-ipd#diff-dG1vbC9zY29yZS9zY29yZV90eXBlcy5weQ==) | `100.00% <100.00%> (ø)` | | | ... and [5 more](https://app.codecov.io/gh/uw-ipd/tmol/pull/254?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uw-ipd) | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

jflat06 commented 1 year ago

@fdimaio Shortening the disulfide params lead to a change of:

E       AssertionError: 
E       Not equal to tolerance rtol=1e-05, atol=1e-05
E       
E       Mismatched elements: 10 / 10 (100%)
E       Max absolute difference: 0.00015092
E       Max relative difference: 4.633452e-05
E        x: array([[-3.257311, -3.257311, -3.257311, -3.257311, -3.257311, -3.257311,
E               -3.257311, -3.257311, -3.257311, -3.257311]], dtype=float32)
E        y: array([[-3.25716, -3.25716, -3.25716, -3.25716, -3.25716, -3.25716,
E               -3.25716, -3.25716, -3.25716, -3.25716]], dtype=float32)

So pretty minimal. Does this look OK?

jflat06 commented 1 year ago

Great, I'll wait for the checks to pass and then merge if they check out.

aleaverfay commented 1 year ago

I just changed the interface to the resolve_uaid function in the rama PR (#252) to take UAIDs as a struct instead of as a TensorAccessor. That doesn't affect this PR as far as I can tell. The dislufide_pose_score.impl.hh file does include uaid_util.hh, but doesn't invoke the functions it contains. It most certainly will affect the cart-bonded term, though, so I wanted to give you the head's up, @jflat06

jflat06 commented 1 year ago

I just changed the interface to the resolve_uaid function in the rama PR (#252) to take UAIDs as a struct instead of as a TensorAccessor. That doesn't affect this PR as far as I can tell. The dislufide_pose_score.impl.hh file does include uaid_util.hh, but doesn't invoke the functions it contains. It most certainly will affect the cart-bonded term, though, so I wanted to give you the head's up, @jflat06

I don't think this actually affects the cart-bonded term. Cart-bonded forgoes the UAID system because it needs access to non-mainline atoms coming from a connection. So part of the implementation was adding a 'paths starting from connection' data structure to each block type.