JOSS Review: Functionality and Examples

[lheagy]

Originally from @jochym in openjournals/joss-reviews#440

• I was able to run command line example, albeit not cleanly. I have got a following warning:

  python2.7/site-packages/histogram/hdf/Loader.py:129: FutureWarning: elementwise comparison failed; returning scalar instead, but in the future will perform elementwise comparison
  if 'storage' in list(dataGroup): # this uses the 'storage' convention

  which does not increase confidence in the code - suggesting that it will soon become obsolete.

• Some (5) tests included in the package fail due to the missing pyversion dependence and there are two warnings. One listed above and the second hard to interpret:

  tests/backward/singlephonon_sqe2dos_TestCase.py::TestCase::test1c
  /home/jochym/Projects/joss-review/multiphonon/multiphonon/backward/singlephonon_sqe2dos.py:125: UserWarning: Scaling factor to combine DOSes calculated is not stable: 22.1354562834 (using continuous criteria) vs 17.7865062045 (using area criteria)
  You may want to consider adjusting the parameters such as E range (Emax more specifically)
  "You may want to consider adjusting the parameters such as E range (Emax more specifically)" % (scale1, scale2)

• The example notebook cannot be executed to the end. The Converting sample data to powder I(Q,E)... stage produced stack dump with following import error:

  ImportError: libjsoncpp.so.0: cannot open shared object file: No such file or directory

  probably due to another missing or changed dependency. Since the notebook is the only fully worked-out example I cannot say the functionality was verified. In fact it will be difficult to even start to fully use the software - having only short, simple and tersely commented example getdos-Al.py to start with.

[yxqd]

@jochym could you please kindly let us know in detail at what kind of system (linux distribution, conda version, etc) your running of tests failed? We would like to reproduce the error and solve the installation problem.

[yxqd]

See also #82

[jochym]

The test system was my working machine. Debian buster with the newest conda on python 3.6 - freshly updated. All tests were run in separate environment created just for the review according to the docs. I am using the same system for all my work and development - which is mostly in conda environments. I'll try to help you by sharing the conda environment file: https://anaconda.org/jochym/review I can also try to repeat my tests on freshly installed virtual machine - but that will have to wait some time.

[yxqd]

@jochym your environment.yml was helpful. I noticed that you have several channels in your configuration beyond what is used for multiphonon. I reproduced that in the .condarc within an ubuntu virtual machine, and it caused installation problems if mantid-framework was installed. After we try to clean up the dependency list of the mantid-framework conda package, this problem seems to go away and I can install mantid-framework without problem using all your channels + multiphonon channels. I don't have a debian buster system to exactly reproduce what you have but I think this probably should solve the installation problem for you too. Please give it a try when you get a chance. Thanks.

[jochym]

Thanks. I will.

[jochym]

As I noted in the https://github.com/sns-chops/multiphonon/issues/82 the clean install in the freshly installed debian desktop machine with clean conda worked as expected. Note however that this is not a typical setup and you cannot expect that users will go to such lengths to run the software. Additionally as the install on my standard system still fails to work. When the software worked, I was able to reproduce expected results. But the examples are still fairly limited and tersly documented.

In summary, this time I was able to at least verify that the expected results can be reproduced. I still think that they are too sparsely documented but they exist and work.

[yxqd]

Thanks @jochym. We are glad that it worked for a standard debian distribution you tried out. Please see my posts in #82 for details. And some documentation improvements are coming: #101