Nose tests fail

[Dongsun Yoo]

I'm not sure I'm supposed to post this here... I have been checking out Amp and got problems testing Amp installation. I have installed the latest Amp development code (3649792) into ubuntu 16.04 + gcc 5.4.0 + python 2.7.12 system. I can run example scripts in the documentation, but nose tests fail.

When I run nosetests ~/path/to/my/codes/amp/tests, I get 2 errors and 3 failures. One error is an AttributeError.

ERROR: numeric_analytic_test.test
    AttributeError: 'list' object has no attribute 'logout'.

This was already mentioned in 348c48c (in the comment). I could fix this by changing amp/model/__init__.py, since it seems we don't open ssh connection to the localhost.

--- a/amp/model/__init__.py 2016-11-22 16:47:04.999047081 +0900
+++ b/amp/model/__init__.py 2016-11-22 16:57:22.667467392 +0900
@@ -482,7 +482,10 @@
                 finished[int(message['id'])] = True

         for _ in self._sessions['connections']:
-            _.logout()
+            try:
+                _.logout()
+            except AttributeError:
+                continue
         del self._sessions['connections']

     def get_loss(self, parametervector, lossprime):

When I fix that, I get 1 error and 4 failures.

ERROR: test_gaussian_tflow.train_test
    Amp did not converge upon training.
FAIL: force_call_test_tflow.periodic_test
    AssertionError: The predicted energy of image 1 is wrong!
FAIL: train_test.non_periodic_0th_bfgs_step_test
    AssertionError: Calculated value of loss function is wrong!
FAIL: train_test.periodic_0th_bfgs_step_test
    AssertionError: Calculated value of loss function is wrong!
FAIL: numeric_analytic_test.test
    AssertionError: The calculated 0 force of atom 0 of image 2 is wrong!

grep diff on the stderr gives

diff at 204 = 0.00356458469469
diff at 414 = 0.00509043299462
diff = 0.131743318856

However, the diff value of numeric_analytic_test changes everytime I run the test.

I also tried nose tests with stable version 0.4.1 (23b597d). It gives 3 failures.

FAIL: train_test.non_periodic_0th_bfgs_step_test
    AssertionError: The calculated value of cost function is wrong!
FAIL: train_test.periodic_0th_bfgs_step_test
    AssertionError: The calculated value of cost function is wrong!
FAIL: train_test.test
    AssertionError: The calculated value of cost function is wrong!

I wonder if this failure indicates that my installation is not working as intended, and that the results obtained from my installation cannot be trusted. Do you have any suggestion on this matter? Which version would you recommend using right now?

[andrew_peterson]

Yes, that bug went in during a "hotfix" that a couple of us cranked out last week at the AIChE meeting to make this work on a DoD account. That commit was b2010d8. So if you check out the prior commit (348c48c) it should work until we get the bug fixed.

@akhorshi should be able to comment on if there are a couple of assertion errors still expected; I believe these were rounding errors out deep in the significant digits, so shouldn't create any serious issues.

[Dongsun Yoo]

Thanks. I'm still getting assertion errors, but I guess that's okay as you said.

By the way, I found that the failure from numeric_analytic_test.py comes from ase. When atoms are fixed by FixAtoms constraint, even if you call set_positions(), the positions of the fixed atoms does not change and get_potential_energy() give the energy of the atoms fixed. This can be resolved by deleting constraints.

--- a/amp/tests/misc_test/numeric_analytic_test.py  2016-11-23 12:57:41.459322176 +0900
+++ b/amp/tests/misc_test/numeric_analytic_test.py  2016-11-23 13:31:28.353013555 +0900
@@ -37,6 +37,7 @@
         newatoms = atoms.copy()
         newatoms.set_calculator(EMT())
         newatoms.get_potential_energy()
+        del newatoms.constraints
         images.append(newatoms)
     return images

I could pass the test by this.

[Alireza Khorshidi]

Thanks for noting the constraint issue! Tests for v0.4 were all passing fine at the time that version was released. Some of the tests for the development version still fail, but like Andy said I believe that is due to the deep significant digits, that might be not a big issue. We are working to fix that.

[andrew_peterson]

Yes, good catch on the constraint issue. I think you are right that this is a bug in ASE and I just opened an issue over there. We will see what they say. In the mean time, we can implement your patch to fix this on our end.

Also, I took care of the logout issue in commit cfbcdfa. I had actually done this several days ago and forgot to push the commit to master! It is there now, so this should be working.

[andrew_peterson]

@Bolsal, I implemented your suggestion in commit a710409. Thanks!

Hopefully the latest development branch should work for you now. Let us know if you spot anything else.

@akhorshi, do we have his remaining question ("AssertionError: Calculated value of loss function is wrong!") covered in another open issue?

[Alireza Khorshidi]

I expect this will be solved as we figure out issue 103: https://bitbucket.org/andrewpeterson/amp/issues/103/reproducibility-in-training

[andrew_peterson]

Remaining problem is listed in another issue, as noted above.