Degiacomi-Lab / molearn

protein conformational spaces meet machine learning
https://degiacomi.org/software/molearn/
GNU General Public License v3.0
40 stars 11 forks source link

[JOSS] Tests #9

Closed RMeli closed 1 year ago

RMeli commented 1 year ago

The only test in the test folder fails with the following error:

Warning: importing 'simtk.openmm' is deprecated.  Import 'openmm' instead.
Dataset.shape: torch.Size([16, 3, 2145])
mean: 44.69796567599069, std: 13.886234009167817
E
======================================================================
ERROR: test_openmm_energy (test_openmm_plugin.Test_openmm_plugin.test_openmm_energy)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/rmeli/git/molearn/test/test_openmm_plugin.py", line 30, in test_openmm_energy
    openmmscore = molearn.openmm_energy(data.mol, data.std, clamp=None, platform = 'CUDA') #xml_file = ['modified_amber_protein.xml',])
                  ^^^^^^^^^^^^^^^^^^^^^
AttributeError: module 'molearn' has no attribute 'openmm_energy'

----------------------------------------------------------------------
Ran 1 test in 0.062s

FAILED (errors=1)

While the functionality of the software can verified through the provided example notebooks and the scripts in the examples folder, I think the code could greatly benefit from some additional unit tests (and, eventually, the use of CI).

This relates to my comment in #8; if tests, examples scripts, and notebooks are not run each time the main package is updated they are bound to diverge eventually.


Since the example_script.py is not testing anything (as far as I can tell), should it be moved elsewhere (for example the examples folder)?


The only necessary action item here (IMO) is to fix the only test available.

From my understanding of JOSS criteria, building a more extensive test suite and adding CI is not required. Personally I'd strongly recommend it as it will benefit future development, but can be added in the future.


https://github.com/openjournals/joss-reviews/issues/5523

degiacom commented 1 year ago

Thank you for your feedback, I agree testing needs to be improved. We have now updated the unittests in test/test_openmm_plugin.py. Concerning example_script.py, it is currently used in the CI of conda-forge.

RMeli commented 1 year ago

Thanks for updating the test and adding some more! I was able to run them with python -m coverage run -m unittest and they all pass.

Ran 32 tests in 7.462s

OK

Coverage report:

Name                                                                               Stmts   Miss  Cover
------------------------------------------------------------------------------------------------------
/home/rmeli/git/molearn/src/molearn/__init__.py                                        5      0   100%
/home/rmeli/git/molearn/src/molearn/data/__init__.py                                   2      0   100%
/home/rmeli/git/molearn/src/molearn/data/pdb_data.py                                 122     25    80%
/home/rmeli/git/molearn/src/molearn/loss_functions/__init__.py                         3      0   100%
/home/rmeli/git/molearn/src/molearn/loss_functions/openmm_thread.py                  262    121    54%
/home/rmeli/git/molearn/src/molearn/loss_functions/torch_protein_energy.py           213    191    10%
/home/rmeli/git/molearn/src/molearn/loss_functions/torch_protein_energy_utils.py     761    740     3%
/home/rmeli/git/molearn/src/molearn/trainers/__init__.py                               5      0   100%
/home/rmeli/git/molearn/src/molearn/trainers/openmm_physics_trainer.py                39     28    28%
/home/rmeli/git/molearn/src/molearn/trainers/sinkhorn_trainer.py                     206    170    17%
/home/rmeli/git/molearn/src/molearn/trainers/torch_physics_trainer.py                 38     27    29%
/home/rmeli/git/molearn/src/molearn/trainers/trainer.py                              202    169    16%
test_openmm_plugin.py                                                                138      1    99%
------------------------------------------------------------------------------------------------------
TOTAL                                                                               1996   1472    26%