Skip to content
This repository has been archived by the owner on Nov 28, 2023. It is now read-only.

Skip empty features #140

Merged
merged 49 commits into from
Apr 10, 2020
Merged

Skip empty features #140

merged 49 commits into from
Apr 10, 2020

Conversation

NicoRenaud
Copy link
Member

handles missing CA/CB atoms in residue based features

  • used the center of the residue for ResidueDensity and BSA
  • Skip them for PSSM

LilySnow and others added 30 commits May 7, 2019 16:36
Ignore Mac .DS_Store file
Added bug report tempalte
Added feature request issue template
set export to False
Bumps [freesasa](https://github.com/mittinatten/freesasa) from 2.0.3.post7 to 2.0.5.
- [Release notes](https://github.com/mittinatten/freesasa/releases)
- [Changelog](https://github.com/mittinatten/freesasa/blob/master/CHANGELOG.md)
- [Commits](https://github.com/mittinatten/freesasa/commits)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
Updates the requirements on [torch](https://github.com/pytorch/pytorch) to permit the latest version.
- [Release notes](https://github.com/pytorch/pytorch/releases)
- [Commits](pytorch/pytorch@v0.1.1...v1.4.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
@coveralls
Copy link

coveralls commented Apr 6, 2020

Pull Request Test Coverage Report for Build 784

  • 115 of 168 (68.45%) changed or added relevant lines in 9 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+2.7%) to 76.343%

Changes Missing Coverage Covered Lines Changed/Added Lines %
deeprank/features/BSA.py 3 4 75.0%
deeprank/features/ResidueDensity.py 3 5 60.0%
deeprank/features/FeatureClass.py 20 26 76.92%
deeprank/generate/DataGenerator.py 79 94 84.04%
deeprank/utils/cal_hitrate_successrate.py 3 32 9.38%
Totals Coverage Status
Change from base Build 689: 2.7%
Covered Lines: 1578
Relevant Lines: 2067

💛 - Coveralls

@LilySnow
Copy link
Contributor

LilySnow commented Apr 6, 2020

Shall we use residue center for all residues instead of for residues with missing atoms?

@NicoRenaud
Copy link
Member Author

it was at the center first but then we decided to move to CB ...

@NicoRenaud
Copy link
Member Author

I suggest we leave the features like that for now, This PR should fix the issue but we have then different default for different features when the PDB is faulty ...

@@ -212,6 +212,14 @@ def get_feature_value(self, cutoff=5.5):
f"{self.mol_name}: The following interface residues have "
f" no pssm value:\n {ctc_res_wo_pssm}"
)
elif len(pssm_res_set.difference(ctc_res_set)) > 0:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pssm_res_set contains pssm for all residues, and ctc_res_set consider only interface residues, so this condition should always be True.
Also pssm_res_wo_ctc = pssm_res_set.difference(ctc_res_set) misstreats the non-interface residues as residues with missing atoms.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is that some of the res are missing in xyz_dict therefore leading to a keyError on line 229. I think we should create a get_residue_centers either in DeepRank or pdb2sql. There we could specify to get either CB, or CA if CB is missing or take the mean of the residue xyz if CA and CB are missing. The only issue is the we would have to loop over the residue and it will be a bit slow for large complexes ....

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need a data preprocess step in DeepRank to avoid these problems. We can could add a PDB validator in pdb2sql.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a good idea ... but I wouldn't know how to do it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've introduced a get_residue_center in FetureClass I think we can live with that for now ....

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the get_residue_center looks good.

Copy link
Member

@CunliangGeng CunliangGeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the changes look good.

@NicoRenaud NicoRenaud merged commit 08fe153 into development Apr 10, 2020
@NicoRenaud NicoRenaud deleted the skip_empty_features branch October 23, 2020 14:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants