Rdf fitting improvement

[orionarcher]

Description

This PR implements a new default solvation cutoff identifier based on scipy.signal.find_peaks. The performance seems to be better than the old method and it is vulnerable to fewer weird gotchas.

Based on @hmacdope's work in the rdf_fitting_demo tutorial.

Once merged, this PR will solve issue #43

Todos

Notable points that this PR has either accomplished or will accomplish.

• [x] implement new rdf parser based on find peaks
• [x] add tests for parser
• [x] add documentation for parser
• [x] update tutorial

Status

• [x] Ready to go

[pep8speaks]

Hello @orionarcher! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file solvation_analysis/rdf_parser.py:

Line 226:21: E251 unexpected spaces around keyword / parameter equals Line 226:23: E251 unexpected spaces around keyword / parameter equals

• In the file solvation_analysis/tests/test_rdf_parser.py:

Line 108:1: E302 expected 2 blank lines, found 1 Line 144:1: E302 expected 2 blank lines, found 1

Comment last updated at 2022-07-13 23:06:40 UTC

[review-notebook-app[bot]]

Check out this pull request on 

See visual diffs & provide feedback on Jupyter Notebooks.

---

Powered by ReviewNB

[codecov[bot]]

Codecov Report

Merging #63 (71381ac) into main (fd9dbd5) will decrease coverage by 0.28%. The diff coverage is 93.75%.

[orionarcher]

Hi @hmacdope, I added a new way of finding the solvation cutoff based on scipy.signal.find_peaks.

This function applies the following procedure to identify peaks.
    1. normalize the RDF
    2. apply a gaussian convolution (std=1.1) to the RDF
    3. call scipy.signal.find_peaks on the RDF and -1*RDF to find the peaks and troughs, respectively.
    4. apply simple heuristics to see if the cutoff is valid
          - trough follows peak
          - in `Solution.cutoff_region` (specified by kwarg)
          - normalized peak height > 0.05

Take a look when you get a chance, I think this is pretty much good to go!

[orionarcher]

Thanks for the comments! I implemented all of your suggested changes. Once this passes tests, I plan to merge.