Add warning about unstable API

[mikemhenry]

RE: #430 (comment)

Once this gets merged, I will get a new release cut and on conda-forge 🎉

[mikemhenry]

@jchodera at some point it looks like CI has stopped working, I did a quick pass at getting it working locally but it uses some deprecated utilities: https://numpy.org/doc/stable/reference/generated/numpy.testing.dec.deprecated.html?highlight=numpy%20testing%20decorators
so it will take a bit of work, I'll make an issue and add it to the todo pile

[jchodera]

Oh, good point---we definitely need to switch to GitHub Actions, and perhaps upgrade the repo to the latest MolSSI Cookiecutter structure at the same time. Please do take a stab at this when able!

[mikemhenry]

@jchodera do we want to get the release out before we get CI fixed up? Or do we want to get this release out ASAP?

[jchodera]

Looks great, thanks!

Let's see if we can get the release cut (if all tests pass locally) and then the conda-forge package updated for @cbayly13!

[mikemhenry]

Okay I was able to run tests locally (requires python 3.7 and numpy 1.17) and everything passes except test_bar_free_energies

If I run it 10 times, it fails 6/10, with errors like:

E           AssertionError: 
E           Arrays are not almost equal to 6 decimals
E            ACTUAL: 0.02220175488519857
E            DESIRED: 0.0222330399700463


E           AssertionError: 
E           Arrays are not almost equal to 6 decimals
E            ACTUAL: 0.021965581661354587
E            DESIRED: 0.02196921013221197

E           AssertionError: 
E           Arrays are not almost equal to 6 decimals
E            ACTUAL: 0.016445521371683907
E            DESIRED: 0.01646759723648925

Is that okay @jchodera ?

[mikemhenry]

I ran it 1000 times:
798 failed, 202 passed

[mrshirts]

Might need to turn up the tolerance of the MBAR iterator? Or the BAR iterator?

[jchodera]

You mean tighten the default relative_tolerance in this line from 1e-7 to 1e-8?

[mrshirts]

Or for the tests pass in a higher tolerance. We probably should have the MBAR and BAR tolerance the same.

[jchodera]

Agreed. That change would occur here, then, and should probably set the number of decimal places to 5 (instead of the current 6).

[mikemhenry]

With these changes (setting BAR and MBAR iterator to the same tolerance and setting the test utility to 5 decimal places):

diff --git a/pymbar/bar.py b/pymbar/bar.py
index 5d9ee98..d8b8ecd 100644
--- a/pymbar/bar.py
+++ b/pymbar/bar.py
@@ -151 +151 @@ def BARzero(w_F, w_R, DeltaF):
-def BAR(w_F, w_R, DeltaF=0.0, compute_uncertainty=True, uncertainty_method='BAR', maximum_iterations=500, relative_tolerance=1.0e-12, verbose=False, method='false-position', iterated_solution=True, return_dict=False):
+def BAR(w_F, w_R, DeltaF=0.0, compute_uncertainty=True, uncertainty_method='BAR', maximum_iterations=500, relative_tolerance=1.0e-8, verbose=False, method='false-position', iterated_solution=True, return_dict=False):
diff --git a/pymbar/mbar.py b/pymbar/mbar.py
index f3eace2..d65c9f1 100644
--- a/pymbar/mbar.py
+++ b/pymbar/mbar.py
@@ -73 +73 @@ class MBAR:
-    def __init__(self, u_kn, N_k, maximum_iterations=10000, relative_tolerance=1.0e-7, verbose=False, initial_f_k=None,
+    def __init__(self, u_kn, N_k, maximum_iterations=10000, relative_tolerance=1.0e-8, verbose=False, initial_f_k=None,
@@ -1062 +1062 @@ class MBAR:
-        
+
diff --git a/pymbar/utils_for_testing.py b/pymbar/utils_for_testing.py
index 299dda8..fb40f27 100644
--- a/pymbar/utils_for_testing.py
+++ b/pymbar/utils_for_testing.py
@@ -97 +97 @@ def get_fn(name):
-def eq(o1, o2, decimal=6, err_msg=''):
+def eq(o1, o2, decimal=5, err_msg=''):

We now have 144 failed, 856 passed for 1000 iterations of test_bar_free_energies. All the other tests still pass.

[mrshirts]

What happens if you bump the relative tolerance up to 10^-9 for both? Do more pass?

[mikemhenry]

Yes that drops it down to only 114 failing. With 10^-12 I get the same amount of failures ~110 (out of 1,000 repeat tests)

[jchodera]

This is a little worrying, since there were no BAR tests until I added them a month ago...

[mikemhenry]

I suppose the bigger question is, do we punt this until we get CI setup? I've been digging into these issues a bit and it looks like the pymbar4 branch fixes quite a few of these issues. There seems to still be a lot to do for pymbar4 (judging from a potentially outdated project board) so it may be best to:

1. Merge this PR in and cut release
2. Port the CI fixes from the pymbar4 branch into master/main
3. Figure out how we will resume the work on the pymbar4 branch

[mrshirts]

I am planning on working on pymbar4 when classes are over (like ~3 weeks). Perhaps we should set up a time to meet then to go over what needs to be done?

[jchodera]

Sounds good! Let's proceed as @mikemhenry suggests in the meantime.