feat: added bad_res_radius feature, allowing to specify cutoff distance

[joanimato]

Description

This feature is addresses #473

A new CLI option, --bad_res_radius is provided that allows the user to specify a cutoff radius (from the box center) outside of which bad residues will be ignored. If the bad residue falls within the specified cutoff radius, an error will be raised as usual. This is meant to be used in conjunction with --allow_bad_res / --delete_bad_res. A box specification is also necessary (or at the very lost, the box_center).

From the API side, the same can be done by passing a bad_res_radius into the Polymer.from_pdb_string() (or from_prody, etc...) functions. This requires that the box_center (as an array) is also defined in the MoleculePreparation object:

mk_prep = MoleculePreparation(box_center =....)

# or 

mk_config["box_center"] = np.array(....)
mk_prep = MoleculePreparation.from_config(mk_config)


# etc....

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Other (please describe):

Checklist:

Please go through the following checklist before submitting the PR:

• Written a description of the feature or bug fix above.
• Ran pytest locally and all tests have passed.
• If applicable, documentation was updated with new feature.
• Docstring included for any new classes/functions/methods, or for significantly modifed existing functions.
• (Optional) new test added to account for new feature.

Additional Information

Diogo suggested an API change to meeko, so this feature may be temporary and will have to be rewritten.

[allisonbarkdull]

could you also make this work with --box_enveloping and have the radius be the distance from the center of whatever the box is enveloping?

[joanimato]

could you also make this work with --box_enveloping and have the radius be the distance from the center of whatever the box is enveloping?

Yes, that should be working.

[allisonbarkdull]

oki, I'll fix the help message then.

To me the feature would be more intuitive if the --bad_res_radius is passed INSTEAD of --delete_bad_res, not in addition to it. Passing --delete_bad_res makes me feel like all bad residues will be deleted even if they are in my radius which freaks me out

[joanimato]

oki, I'll fix the help message then.

To me the feature would be more intuitive if the --bad_res_radius is passed INSTEAD of --delete_bad_res, not in addition to it. Passing --delete_bad_res makes me feel like all bad residues will be deleted even if they are in my radius which freaks me out

yes, that makes sense

[joanimato]

On a second thought, you are still deleting bad residues, just outside of a given radius, so maybe it makes sense to have both?

You can assume that they always are passed together, except that when you don't specify a radius, it defaults to 0. We can even add in the docs that if a radius is not passed, that's equivalent to passing a radius of 0 (also modify the help doc for --delete_bad_radius)

Perhaps @diogomart can weigh in.

[allisonbarkdull]

I had envisioned this feature to allow bad residues outside a radius extending from the search box, nor a radius from the center of mass of the ligand/box. Using a specrical volume from the ligand/box's COM is problematic for irregularly shaped or elongated ligands: a long, skinny ligand produces a box that is much larger in one dimension than another, so a fixed radius from the center is a poor proxy for "is this residue actually outside the docking search spave?"

Drafted a version with this implemented, would appreciate feedback

[allisonbarkdull]

I also changed it so you can't pass --bad_res_radius & --delete_bad_res together. This is more intuitive to me as a user, but I also don't feel strongly and I think that either way is fine as long as it is clearly documented w appropriate warnings.
feel free to switch it if you feel super strongly

[diogomart]

Made the following changes:

1. Renamed option to --delete_bad_res_from_box_radius. It's now independent and incompatible with --delete_bad_res both in API and CLI.
2. Removed duplicated code to parse file passed to box enveloping.
3. The test now tests that an error is raised if cutoff is larger than min distance to residue
4. Compute minimum distance between all atoms of each residue and box surface
5. Removed box info from MoleculePreparation (this class is Polymer agnostic).

[joanimato]

hmm, not crazy about that name. Too long and too many underscores.

[diogomart]

Fair point, --delete_bad_res_radius came to mind but the from_box part adds valuable info. I think clarity outweighs typing speed as far more time is spent understanding what the code does than typing the command. If we can find a better alternative that would be awesome, but I couldn't.

[diogomart]

Another perspective is that we are consolidating two commands in one, with more info (from box), and in about the same length

--delete_bad_res_from_box_radius 5
--delete_bad_res --bad_res_radius 5

[allisonbarkdull]

the only thing I can think is --delete_bad_res_outsise_box_radius is slightly more clear. but I agree w @diogomart that this is just a challenging one - its confusing having a box radius and then this additional radius outside the box radius.

maybe there was a simpler way to implement this feature, but this is the most useful way I could come up with

totally open to your suggestions tho @joanimato

otherwise looks great to me. thanks for the thorough review @diogomart

[allisonbarkdull]

maybe --delete_bad_res --bad_res_distance_from_box
is most intuitive ? i think radius should be changed to distance since radius implies a spherical area and we are deleting bad residues outside a distance from the box

[diogomart]

@allisonbarkdull both sound good to me

[diogomart]

Merging, but happy to optimize the choice of option names in future PRs.