Skip to content
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.

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

Already on GitHub? Sign in to your account

More consistently use existing charge caching #1115

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mattwthompson
Copy link
Member

@mattwthompson mattwthompson commented Nov 21, 2024

Description

A few code paths didn't use the improvements of #1066 / #1069, which caused some horrible performance in some number of corner cases. (Writing out GROMACS files in the protein-ligand example took 10 minutes (!) compared to a few seconds for OpenMM.)

I'm more than a little confused as to how from_openmm code paths might have been hit in this example - which just uses SMIRNOFF force fields and Interchange.combine - but I could be misremembering which changes actually affected things.

Checklist

  • Add tests
  • Lint
  • Update docstrings

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link

codecov bot commented Nov 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.39%. Comparing base (dec585e) to head (396d472).

Additional details and impacted files
---- 🚨 Try these New Features:

@mattwthompson
Copy link
Member Author

Upstream:

============================= slowest 20 durations =============================
474.12s call     examples/protein_ligand/protein_ligand.ipynb::Cell 20
427.49s call     examples/protein_ligand/protein_ligand.ipynb::Cell 22
188.02s call     examples/protein_ligand/protein_ligand.ipynb::Cell 26
95.74s call     examples/ligand_in_water/ligand_in_water.ipynb::Cell 14
85.04s call     examples/ligand_in_water/ligand_in_water.ipynb::Cell 18
69.02s call     examples/packed_box/packed_box.ipynb::Cell 6
65.37s call     examples/ligand_in_water/ligand_in_water.ipynb::Cell 8
62.18s call     examples/protein_ligand/protein_ligand.ipynb::Cell 7
59.11s call     examples/ligand_in_water/ligand_in_water.ipynb::Cell 6
32.89s call     examples/lammps/lammps.ipynb::Cell 6
30.41s call     examples/amber/amber.ipynb::Cell 4
30.02s call     examples/host-guest/host_guest.ipynb::Cell 6
27.48s call     examples/protein_ligand/protein_ligand.ipynb::Cell 9
20.64s call     examples/openmm/openmm.ipynb::Cell 5
19.13s call     examples/protein_ligand/protein_ligand.ipynb::Cell 18
18.69s call     examples/openmm/openmm.ipynb::Cell 4
14.53s call     examples/packed_box/packed_box.ipynb::Cell 4
14.51s call     examples/ligand_in_water/ligand_in_water.ipynb::Cell 19
12.16s call     examples/openmm/openmm.ipynb::Cell 0
11.10s call     examples/amber/amber.ipynb::Cell 0
================= 119 passed, 3 skipped in 1247.79s (0:20:47) ==================

These changes:

============================= slowest 20 durations =============================
89.78s call     examples/ligand_in_water/ligand_in_water.ipynb::Cell 14
64.33s call     examples/ligand_in_water/ligand_in_water.ipynb::Cell 18
63.69s call     examples/ligand_in_water/ligand_in_water.ipynb::Cell 8
63.53s call     examples/protein_ligand/protein_ligand.ipynb::Cell 7
57.87s call     examples/ligand_in_water/ligand_in_water.ipynb::Cell 6
38.[25](https://github.com/openforcefield/openff-interchange/actions/runs/11958262648/job/33337380981?pr=1115#step:8:26)s call     examples/packed_box/packed_box.ipynb::Cell 6
33.39s call     examples/lammps/lammps.ipynb::Cell 6
30.02s call     examples/host-guest/host_guest.ipynb::Cell 6
29.95s call     examples/protein_ligand/protein_ligand.ipynb::Cell 22
28.00s call     examples/amber/amber.ipynb::Cell 4
[26](https://github.com/openforcefield/openff-interchange/actions/runs/11958262648/job/33337380981?pr=1115#step:8:27).46s call     examples/protein_ligand/protein_ligand.ipynb::Cell 9
25.93s call     examples/protein_ligand/protein_ligand.ipynb::Cell 26
21.18s call     examples/openmm/openmm.ipynb::Cell 5
19.35s call     examples/openmm/openmm.ipynb::Cell 4
17.60s call     examples/protein_ligand/protein_ligand.ipynb::Cell 18
15.10s call     examples/packed_box/packed_box.ipynb::Cell 4
11.91s call     examples/openmm/openmm.ipynb::Cell 0
11.38s call     examples/lammps/lammps.ipynb::Cell 0
11.18s call     examples/amber/amber.ipynb::Cell 0
9.00s call     examples/packed_box/packed_box.ipynb::Cell 2
================== 119 passed, 3 skipped in 304.15s (0:05:04) ==================

@mattwthompson mattwthompson marked this pull request as ready for review November 21, 2024 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant