incorporating pKa-ANI into pdb2pqr

[sastrys1]

Finally, incorporated pKa-ANI with pdb2pqr and added three test PDBs into the tests folder.

I thought testing this at pH 5.0 would be a good idea because the vast majority of the histidines should be protonated and vast majority of ASP/GLU should be deprotonated. pKa-ANI is able to do this slightly more consistently than propka.

On my list now is to incorporate energy minimization into pKa-ANI's calculation - @kiyoon @sobolevnrm would you want this feature in PDB2PQR? Energy minimization does have a non-trivial impact on pKa-ANI's results

[sastrys1]

@kiyoon @sobolevnrm @seankhl Has anyone been able to take a look at this? I was very excited to share this with the community and I'm bummed it was met with radio silence, but I understand people are busy.

Would one of you be able to start the CI testing workflows so I can test the code if necessary? It would be best to start the CI workflows now so I can get started as early as possible on changing anything that breaks in the CI workflows.

[sobolevnrm]

It looks like two linting tests failed. Can you please fix those and then I will review? This code is now a hobby for me, so I maintain it when I have time—which is less than I’d like. Sorry for the delays.

[kiyoon]

How to fix those formatting issues is described here: https://github.com/Electrostatics/pdb2pqr/blob/master/CONTRIBUTING.md#do-not-submit-messy-code

[sastrys1]

Made the changes, should pass the tests now hopefully!

[sastrys1]

@sobolevnrm CI checks have now passed - I wrote some test code that should hopefully make review easier, comparing the protonation state assignment of pKa-ANI with that of propka on a couple of PDB files

[sastrys1]

@sobolevnrm Have you been able to take a look?

[sobolevnrm]

@sobolevnrm Have you been able to take a look?

No.

[sobolevnrm]

After installing with pip install -e . and attempting to run, I get the warning:

/usr/local/python/3.12.1/lib/python3.12/site-packages/torchani/aev.py:16: UserWarning: cuaev not installed
  warnings.warn("cuaev not installed")

Full example:

@sobolevnrm ➜ /workspaces/pdb2pqr (pka-ani_py310) $ pdb2pqr
/usr/local/python/3.12.1/lib/python3.12/site-packages/torchani/aev.py:16: UserWarning: cuaev not installed
  warnings.warn("cuaev not installed")
/usr/local/python/3.12.1/lib/python3.12/site-packages/torchani/__init__.py:61: UserWarning: Dependency not satisfied, torchani.data will not be available
  warnings.warn("Dependency not satisfied, torchani.data will not be available")
usage: pdb2pqr [-h] [--ff {AMBER,CHARMM,PARSE,TYL06,PEOEPB,SWANSON}] [--userff USERFF] [--clean] [--nodebump] [--noopt] [--keep-chain] [--assign-only]
               [--ffout {AMBER,CHARMM,PARSE,TYL06,PEOEPB,SWANSON}] [--usernames USERNAMES] [--apbs-input APBS_INPUT] [--pdb-output PDB_OUTPUT] [--ligand LIGAND]
               [--whitespace] [--neutraln] [--neutralc] [--drop-water] [--include-header] [--titration-state-method {propka,pkaani}] [--with-ph PH] [-f FILENAMES]
               [-r REFERENCE] [-c CHAINS] [-i TITRATE_ONLY] [-t THERMOPHILES] [-a ALIGNMENT] [-m MUTATIONS] [-p PARAMETERS]
               [--log-level {DEBUG,INFO,WARNING,ERROR,CRITICAL}] [-o PH] [-w WINDOW WINDOW WINDOW] [-g GRID GRID GRID] [--mutator MUTATOR]
               [--mutator-option MUTATOR_OPTIONS] [-d] [-l] [-k] [-q] [--protonate-all] [--version]
               input_path output_pqr
pdb2pqr: error: the following arguments are required: input_path, output_pqr

[sobolevnrm]

Please see my requested changes. I'd also recommend adding some documentation on usage (including reasons why people should choose pkaANI) -- we can start a conversation thread in the PR about how best to do this.

[sastrys1]

@sobolevnrm In response to your observation of the warnings -

The first one:

/usr/local/python/3.12.1/lib/python3.12/site-packages/torchani/aev.py:16: UserWarning: cuaev not installed
  warnings.warn("cuaev not installed")

requires a fairly involved installation of torchani cuaev (given here). The GPU speedup isn't necessary and isn't used in pKa-ANI so we can safely ignore this one imo, or downgrade the dependency to torchani==2.2.0 which won't throw the same error on import.

The second one:

/usr/local/python/3.12.1/lib/python3.12/site-packages/torchani/__init__.py:61: UserWarning: Dependency not satisfied, torchani.data will not be available
  warnings.warn("Dependency not satisfied, torchani.data will not be available")

Simply requires installation of h5py as a dependency, I can add that to pKa-ANI if we want but pKa-ANI doesn't use torchani.data either so we can also just safely ignore it.

[sastrys1]

Please see my requested changes. I'd also recommend adding some documentation on usage (including reasons why people should choose pkaANI) -- we can start a conversation thread in the PR about how best to do this.

Added some documentation and a reference to the paper about why people should use pKa-ANI and how the model was built

[sobolevnrm]

@sobolevnrm In response to your observation of the warnings -

The first one:

/usr/local/python/3.12.1/lib/python3.12/site-packages/torchani/aev.py:16: UserWarning: cuaev not installed
  warnings.warn("cuaev not installed")

requires a fairly involved installation of torchani cuaev (given here). The GPU speedup isn't necessary and isn't used in pKa-ANI so we can safely ignore this one imo, or downgrade the dependency to torchani==2.2.0 which won't throw the same error on import.

The second one:

/usr/local/python/3.12.1/lib/python3.12/site-packages/torchani/__init__.py:61: UserWarning: Dependency not satisfied, torchani.data will not be available
  warnings.warn("Dependency not satisfied, torchani.data will not be available")

Simply requires installation of h5py as a dependency, I can add that to pKa-ANI if we want but pKa-ANI doesn't use torchani.data either so we can also just safely ignore it.

In that case, can you suppress the warning with warnings.filterwarnings?

[sastrys1]

Yes, just pushed a commit in which I filtered the warnings. Also now the warnings only come up if we actually use pKa-ANI - because of @kiyoon's request to create a separate install like pdb2pqr[pkaani] I moved the pkaani imports to inside the run_pkaani() function.

[seankhl]

I haven't taken a close look but at a glance (and as a third party to all of this work) the current implementation looks solid to me. Thanks for your work on this, @sastrys1!

[sastrys1]

@seankhl Thank you so much!

@sobolevnrm Sorry for the delay, reformatted the file with ruff and now everything should be good to go.

[sastrys1]

Is there anything remaining I need to do to get this pull request merged in and get it released as part of PDB2PQR?

[sobolevnrm]

@kiyoon do you want to review before I merge this?

[kiyoon]

I read the changes briefly, looks good to me, but I haven't gotten a chance to really test it

[sobolevnrm]

@sastrys1 it doesn't look like the tests you added are actually running?

python -m pytest tests/pkaani_tests/test_pHs_pkaani_propka.py 
========================================================================= test session starts =========================================================================
platform linux -- Python 3.12.1, pytest-8.4.1, pluggy-1.6.0
rootdir: /workspaces/pdb2pqr
configfile: pyproject.toml
plugins: anyio-4.9.0
collected 0 items                                                                                                                                                     

======================================================================== no tests ran in 0.01s ========================================================================

Otherwise, things seem to be working fine. I'll accept this PR once we get tests working, unless @kiyoon wants to take a closer look at it.

[sastrys1]

@kiyoon @sobolevnrm Apologies for the long delay in response - was caught up with other research responsibilities.

I rewrote the tests to mirror the propka pytest framework with some PDB files, a few local files and one pulling from the Protein Data Bank. I ran the tests using the command python -m pytest pkaani_test.py from the tests directory.

[sastrys1]

@sobolevnrm @kiyoon Have you been able to see if the tests are working now?

[sobolevnrm]

Unfortunately, I have not. It is on my to-do list. Thanks for your patience.

[sobolevnrm]

@sastrys1 this has been merged. Thank you for your changes and my apologies for the delays.

[seankhl]

Any chance for a release to be made for this very nice feature?

[sobolevnrm]

Yes, when I find time. We are always looking for volunteers to help with this effort...

[sobolevnrm]

For example, this merge seems to have broken our build pipeline. It would be great to:

• Upgrade our pipeline to versions of Python from this decade
• Figure out what broke our pipeline on this merge
• Add pipeline building/testing to our PR process

[sastrys1]

@sobolevnrm I believe the pipeline broke here because the pkaani test cases require pip install pdb2pqr[pkaani] - how do we incorporate that into the build pipeline github action? Also, a couple of small things triggered the ruff linter - pushed those fixes to my local fork, can re-open a PR.

I think if we included pip install pdb2pqr[pkaani] as a line in the actions pipeline the other error would be resolved.

[kiyoon]

We can install with pkaani group dependencies, yes, but make sure you install torch in CPU version because installing GPU version will drastically slow down the pipeline for no reason.

We can use uv pip to install instead of pip. See https://docs.astral.sh/uv/guides/integration/pytorch/

We just need to add UV_TORCH_BACKEND=cpu env variable when we install packages with uv pip, and it will choose the correct wheel for installation.

[adnaksskanda]

@kiyoon Thanks for the guidance - I made an edit to the python-package.yml file in the .github/workflows section to try to get this done, and opened a new PR here #443. Please let me know if I did this right or if I should make any changes!