HydrogenBondAutoCorrel uses deprecated features of numpy

[jbarnoud]

Expected behavior

Running the tests throws no warning, and the library does not use deprecated features of the libraries it depends on.

Actual behavior

Running the tests warns about HydrogenBondAutoCorrel using deprecated features of numpy:

MDAnalysisTests/analysis/test_hydrogenbondautocorrel.py::TestHydrogenBondAutocorrel::test_continuous_excl
MDAnalysisTests/analysis/test_hydrogenbondautocorrel.py::TestHydrogenBondAutocorrel::test_continuous_excl
MDAnalysisTests/analysis/test_hydrogenbondautocorrel.py::TestHydrogenBondAutocorrel::test_intermittent_excl
MDAnalysisTests/analysis/test_hydrogenbondautocorrel.py::TestHydrogenBondAutocorrel::test_intermittent_excl
  /home/jon/dev/mdanalysis/package/MDAnalysis/analysis/hbonds/hbond_autocorrel.py:430: DeprecationWarning: elementwise comparison failed; this will raise an error in the future.
    pair = np.delete(pair, np.where(pair==exclude), 0)

MDAnalysisTests/analysis/test_hydrogenbondautocorrel.py::TestHydrogenBondAutocorrel::test_continuous_excl
MDAnalysisTests/analysis/test_hydrogenbondautocorrel.py::TestHydrogenBondAutocorrel::test_intermittent_excl
  <__array_function__ internals>:5: DeprecationWarning: Calling nonzero on 0d arrays is deprecated, as it behaves surprisingly. Use `atleast_1d(cond).nonzero()` if the old behavior was intended. If the context of this warning is of the form `arr[nonzero(cond)]`, just use `arr[cond]`.

Code to reproduce the behavior

pytest analysis/test_hydrogenbondautocorrel.py

Current version of MDAnalysis

• Which version are you using? (run python -c "import MDAnalysis as mda; print(mda.__version__)") 2.0.0-dev0 64a7c05
• Which version of Python (python -V)? Python 3.8.5
• Which operating system? Ubuntu 20.04 on WSL

[IAlibay]

So I didn't really keep up with the happenings of #2791, but the main comment does include:

"This function can also replace the hbonds.hbond_autocorrel module. However, we haven't deprecated this module here as that is being addressed by PR #2746"

It might be that this can all be removed for v2.0 (assuming we deprecate for 1.01)?

Maybe @bieniekmateusz and/or @p-j-smith know better here?

[p-j-smith]

Sorry for the confusion - perhaps @bieniekmateusz and I should have deprecated/removed hbonds.hbond_autocorrel in #2791?

After taking another look at hbonds.hbond_autocorrel, I see it also fits a bi- or tri-exponential decay function to the hydrogen bond lifetime. I'm slowly writing a tutorial for using hydrogenbonds and will include an example of fitting a curve to the hydrogen bond lifetime. This would perhaps give more flexibility to the user in terms of which function to fit, but if people think it would be better to have this functionality added to hydrogenbonds.HBA.hbond_lifetime then I'm happy to add it.

Other than this, it should be okay to do as @IAlibay suggests and remove for 2.0 and deprecate for 1.01.

[IAlibay]

@p-j-smith @bieniekmateusz Just catching up here as we are planning to aim for a final 1.x release at the end of this week. Are we ok deprecating hbonds.hbond_autocorrel, i.e. are you likely to get all the planned changes you want in for 2.0 (our deadline for 2.0 will be ~ 2 months from now)?

[p-j-smith]

Hi @IAlibay, it should be okay to go ahead and deprecate hbonds.hbond_autocorrel. We've not added the curve-fitting functionality to the class, but there is an example of how to do this in a PR for the UserGuide (MDAnalysis/UserGuide#133).

[IAlibay]

Thanks, for the quick response @p-j-smith!

[IAlibay]

pinging @richardjgowers here, apparently there might be a missing overlap in functionality between the two codes? If so we should just fix hbond_autocorrel.

[IAlibay]

After discussions with @richardjgowers the plan will instead be to:
a) move hbond_autocorrel to the hydrogenbonds folder
b) fix the lines which is causing this numpy warning

[IAlibay]

Fixed in #3242 - needs forward porting