8000 chore: more informative message for empty samples after filtering by josejimenezluna · Pull Request #131 · microsoft/bioemu · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

chore: more informative message for empty samples after filtering #131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally se 8000 nd you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

chore: more informative message for empty samples after filtering #131

wants to merge 18 commits into from

Conversation

josejimenezluna
Copy link
Contributor

We've had a few users report about the default --filter_samples=True behaviour. Currently it fails on the mdtraj.superpose call after samples have been filtered, which is not very user friendly.

Instead here we add an explicit assertion to check that the number of frames in the trajectory is greater than 0 after filtering. The error message should be more informative.

@josejimenezluna josejimenezluna requested a review from a team May 20, 2025 12:47
sarahnlewis
sarahnlewis previously approved these changes May 20, 2025
View reviewed changes
src/bioemu/convert_chemgraph.py Outdated
@@ -453,6 +453,9 @@ def save_pdb_and_xtc(
f"Filtered {num_samples_unfiltered} samples down to {len(traj)} "
"based on structure criteria. Filtering can be disabled with `--filter_samples=False`."
)
assert (
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could also tell the user:

  • Where to find their unphysical samples
  • How to try a different sampling algorithm/number of denoising steps

@ryotatomioka
Copy link
Member
ryotatomioka commented May 20, 2025 •
edited
Loading

How about returning the original unphysical samples (and print out a warning) if the filtering resulted in zero samples?

@github-actions GitHub Actions
Copy link

Summary

Summary
Generated on: 05/20/2025 - 13:05:59
Parser: Cobertura
Assemblies: 5
Classes: 22
Files: 22
Line coverage: 66.1% (1643 of 2483)
Covered lines: 1643
Uncovered lines: 840
Coverable lines: 2483
Total lines: 8230
Covered branches: 0
Total branches: 0
Method coverage: Feature is only available for sponsors

Coverage

src.bioemu - 78.8%
Name Line Branch
src.bioemu 78.8% ****
init.py 100%
chemgraph.py 100%
convert_chemgraph.py 68.4%
denoiser.py 96.8%
get_embeds.py 68.6%
md_utils.py 85.8%
models.py 94.1%
run_hpacker.py 0%
sample.py 44.3%
sde_lib.py 84.3%
seq_io.py 93.9%
shortcuts.py 100%
sidechain_relax.py 77.4%
so3_sde.py 81%
structure_module.py 99%
utils.py 59.3%
< 8000 summary>src.bioemu.colabfold_setup -
Name Line Branch
src.bioemu.colabfold_setup **** ****
init.py
src.bioemu.hpacker_setup - 58.8%
Name Line Branch
src.bioemu.hpacker_setup 58.8% ****
init.py
setup_hpacker.py 58.8%
src.bioemu.openfold.np - 44%
Name Line Branch
src.bioemu.openfold.np 44% ****
protein.py 31.2%
residue_constants.py 60.7%
src.bioemu.openfold.utils - 50.1%
Name Line Branch
src.bioemu.openfold.utils 50.1% ****
rigid_utils.py 50.1%

@github-actions GitHub Actions
Copy link

Summary

Summary
Generated on: 05/20/2025 - 14:42:46
Parser: Cobertura
Assemblies: 5
Classes: 22
Files: 22
Line coverage: 66% (1643 of 2486)
Covered lines: 1643
Uncovered lines: 843
Coverable lines: 2486
Total lines: 8242
Covered branches: 0
Total branches: 0
Method coverage: Feature is only available for sponsors

Coverage

src.bioemu - 78.7%
Name Line Branch
src.bioemu 78.7% ****
init.py 100%
chemgraph.py 100%
convert_chemgraph.py 67.1%
denoiser.py 96.8%
get_embeds.py 68.6%
md_utils.py 85.8%
models.py 94.1%
run_hpacker.py 0%
sample.py 44.3%
sde_lib.py 84.3%
seq_io.py 93.9%
shortcuts.py 100%
sidechain_relax.py 77.4%
so3_sde.py 81%
structure_module.py 99%
utils.py 59.3%
src.bioemu.colabfold_setup -
Name Line Branch
src.bioemu.colabfold_setup **** ****
init.py
src.bioemu.hpacker_setup - 58.8%
Name Line Branch
src.bioemu.hpacker_setup 58.8% ****
init.py
setup_hpacker.py 58.8%
src.bioemu.openfold.np - 44%
Name Line Branch
src.bioemu.openfold.np 44% ****
protein.py 31.2%
residue_constants.py 60.7%
src.bioemu.openfold.utils - 50.1%
Name Line Branch
src.bioemu.openfold.utils 50.1% ****
rigid_utils.py 50.1%

@github-actions GitHub Actions
Copy link

Summary

Summary
Generated on: 05/20/2025 - 14:44:48
Parser: Cobertura
Assemblies: 5
Classes: 22
Files: 22
Line coverage: 66% (1643 of 2487)
Covered lines: 1643
Uncovered lines: 844
Coverable lines: 2487
Total lines: 8243
Covered branches: 0
Total branches: 0
Method coverage: Feature is only available for sponsors

Coverage

src.bioemu - 78.6%
Name Line Branch
src.bioemu 78.6% ****
init.py 100%
chemgraph.py 100%
convert_chemgraph.py 66.6%
denoiser.py 96.8%
get_embeds.py 68.6%
md_utils.py 85.8%
models.py 94.1%
run_hpacker.py 0%
sample.py 44.3%
sde_lib.py 84.3%
seq_io.py 93.9%
shortcuts.py 100%
sidechain_relax.py 77.4%
so3_sde.py 81%
structure_module.py 99%
utils.py 59.3%
src.bioemu.colabfold_setup -
Name Line Branch
src.bioemu.colabfold_setup **** ****
init.py
src.bioemu.hpacker_setup - 58.8%
Name Line Branch
src.bioemu.hpacker_setup 58.8% ****
init.py
setup_hpacker.py 58.8%
src.bioemu.openfold.np - 44%
Name Line Branch
src.bioemu.openfold.np 44% ****
protein.py 31.2%
residue_constants.py 60.7%
src.bioemu.openfold.utils - 50.1%
Name Line Branch
src.bioemu.openfold.utils 50.1% ****
rigid_utils.py 50.1%

@github-actions GitHub Actions
Copy link

Summary

Summary
Generated on: 05/20/2025 - 14:50:47
Parser: Cobertura
Assemblies: 5
Classes: 22
Files: 22
Line coverage: 66% (1643 of 2487)
Covered lines: 1643
Uncovered lines: 844
Coverable lines: 2487
Total lines: 8242
Covered branches: 0
Total branches: 0
Method coverage: Feature is only available for sponsors

Coverage

src.bioemu - 78.6%
Name Line Branch
src.bioemu 78.6% ****
init.py 100%
chemgraph.py 100%
convert_chemgraph.py 66.6%
denoiser.py 96.8%
get_embeds.py 68.6%
md_utils.py 85.8%
models.py 94.1%
run_hpacker.py 0%
sample.py 44.3%
sde_lib.py 84.3%
seq_io.py 93.9%
shortcuts.py 100%
sidechain_relax.py 77.4%
so3_sde.py 81%
structure_module.py 99%
utils.py 59.3%
src.bioemu.colabfold_setup -
Name Line Branch
src.bioemu.colabfold_setup **** ****
init.py
src.bioemu.hpacker_setup - 58.8%
Name Line Branch
src.bioemu.hpacker_setup 58.8% ****
init.py
setup_hpacker.py 58.8%
src.bioemu.openfold.np - 44%
Name Line Branch
src.bioemu.openfold.np 44% ****
protein.py 31.2%
residue_constants.py 60.7%
src.bioemu.openfold.utils - 50.1%
Name Line Branch
src.bioemu.openfold.utils 50.1% ****
rigid_utils.py 50.1%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sarahnlewis sarahnlewis sarahnlewis left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@josejimenezluna @ryotatomioka @sarahnlewis
0