Joss-reviews: [REVIEW]: Ensight4Matlab: read, process, and write data in EnSight Gold format from C++ or MATLAB

[whedon]

Submitting author: schmeisser (Andre Schmeißer)
Repository: https://github.com/ITWM-TVFS/Ensight4Matlab
Version: v1.0
Editor: @cMadan
Reviewer: @Kevin-Mattheus-Moerman
Archive: 10.5281/zenodo.1095608

Status

Status badge code:

HTML: <a href="http://joss.theoj.org/papers/63e2a265afe07dff7564dfacb6086a72"><img src="http://joss.theoj.org/papers/63e2a265afe07dff7564dfacb6086a72/status.svg"></a>
Markdown: [![status](http://joss.theoj.org/papers/63e2a265afe07dff7564dfacb6086a72/status.svg)](http://joss.theoj.org/papers/63e2a265afe07dff7564dfacb6086a72)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer questions

Conflict of interest

• [x] As the reviewer I confirm that there are no conflicts of interest for me to review this work (such as being a major contributor to the software).

General checks

• [x] Repository: Is the source code for this software available at the repository url?
• [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• [x] Version: Does the release version given match the GitHub release (v1.0)?
• [x] Authorship: Has the submitting author (schmeisser) made major contributions to the software?

Functionality

• [x] Installation: Does installation proceed as outlined in the documentation?
• [x] Functionality: Have the functional claims of the software been confirmed?
• [x] Performance: Have any performance claims of the software been confirmed?

Documentation

• [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g. API method documentation)?
• [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
• [x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

• [x] Authors: Does the paper.md file include a list of authors with their affiliations?
• [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [x] References: Do all archival references that should have a DOI list one (e.g. papers, datasets, software)?

[whedon]

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks for JOSS. @Kevin-Mattheus-Moerman it looks like you're currently assigned as the reviewer for this paper :tada:.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As as reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all JOSS reviews 😿

To fix this do the following two things:

1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

For a list of things I can do to help you, just type:

@whedon commands

[Kevin-Mattheus-Moerman]

@schmeisser I started the review process. As I said my review will largely focus on the MATLAB integration. I have little experience with C++ so we will find additional reviewers for that component of your work.
I may need some help with installation. I've been following the installation instructions but got stuck (might be doing something wrong) building the EnsightLib library. I use Ubuntu 16.04 64-bit, QMake version 2.01a, Qt version 4.8.7.
If I run:

 qmake-qt4 EnsightLib.pro
make

I get the following message:

g++ -c -m64 -pipe -std=c++11 -Wall -Wextra -pedantic -O2 -Wall -W -D_REENTRANT -fPIC -DQT_NO_DEBUG -DQT_CORE_LIB -I/usr/share/qt4/mkspecs/linux-g++-64 -I. -I/usr/include/qt4/QtCore -I/usr/include/qt4 -I/usr/local/include -Ilib/.moc -o lib/.obj/ensightlib.o src/ensightlib.cpp
src/ensightlib.cpp:26:19: fatal error: QString: No such file or directory
compilation terminated.
Makefile:261: recipe for target 'lib/.obj/ensightlib.o' failed
make: *** [lib/.obj/ensightlib.o] Error 1

[schmeisser]

@Kevin-Mattheus-Moerman
Thank you for your time reviewing.

Do you have the development headers package installed for Qt 4?
The missing file _QString_ is supposed to be found in _/usr/include/qt4/QtCore_, which is correctly included in your command's output, so I assume the file is not installed.

I have just tried to reproduce the issue with a clean Ubuntu 16.04 VM. The required headers are available in the package libqt4-dev. From a clean state, I succeeded building the library with the following commands.

sudo apt-get install git libqt4-dev libeigen3-dev
git clone https://github.com/ITWM-TVFS/Ensight4Matlab
cd Ensight4Matlab/ensight_lib
qmake-qt4 EnsightLib.pro
make

The one issue I ran into: the include headers for the Eigen library on Ubuntu where not installed where I expected them to be, so I updated the file _EnsightLib.pro_ in the meantime. This is already reflected in the repository so I hope it should not give you any troubles.

---

Once you have succeeded building the library, please note that the build process produces an .so file "lib/libEnsightLib.so*". At this point, the file is local to you and not automatically found by the system. You can either copy it to a globally accessible location like this

sudo cp lib/lbEnsightLib.so* /lib

or define the LD_LIBRARY_PATH environment variable to find your local file

export LD_LIBRARY_PATH=/home/USER/Ensight4Matlab/ensight_lib/lib/:${LD_LIBRARY_PATH}

as described in https://github.com/ITWM-TVFS/Ensight4Matlab#building-the-matlab-interface

You should then be able to build and run the EnsightViewer C++ example.

Hope this helps, best regards.

[Kevin-Mattheus-Moerman]

@schmeisser Thanks. I got a couple of steps further. I'm having some (probably basic) issues with runmex. Btw I also did
export LD_LIBRARY_PATH=/home/kevin/Desktop/JOSS_review/2017_03_22_Ensight4Matlab/repo/Ensight4Matlab/ensight_lib/lib/:${LD_LIBRARY_PATH}

Using SETUP.m I set the paths as:

PATH settings
  QT(header)            :    /usr/include/qt4
  QT(lib)           :    /usr/lib/x86_64-linux-gnu/
  Eigen         :    /home/kevin/Desktop/JOSS_review/2017_03_22_Ensight4Matlab/3rd/eigen/
  Ensight(header)   :    /home/kevin/Desktop/JOSS_review/2017_03_22_Ensight4Matlab/repo/Ensight4Matlab/ensight_lib/include/
  Ensight(lib)      :    /home/kevin/Desktop/JOSS_review/2017_03_22_Ensight4Matlab/repo/Ensight4Matlab/ensight_lib/lib/

Which seems correct. When I run runmex.m I get a warning (

Building with 'g++'.
Warning: You are using gcc version '5.4.1'. The version of gcc is not supported. The version currently supported with MEX is '4.9.x'. For a list of currently supported compilers see:
http://www.mathworks.com/support/compilers/current_release. 
> In runmex (line 62) 

) but it does seem to complete successfully. When I try the examples or
>> newData = EnsightLib;
I get:

Warning: The following error was caught while executing 'EnsightLib' class destructor:
Invalid MEX-file '/home/kevin/Desktop/JOSS_review/2017_03_22_Ensight4Matlab/repo/Ensight4Matlab/EnsightMatlab/EnsightLib_interface.mexa64': libEnsightLib.so.1: cannot open shared object file:
No such file or directory.

[schmeisser]

@Kevin-Mattheus-Moerman, there seem to be two issues here.

The second error message complains that the file libEnsightLib.so.1 cannot be found. If you have started MATLAB with the LD_LIBRARY_PATH variable set, it should look for the file /home/kevin/Desktop/JOSS_review/2017_03_22_Ensight4Matlab/repo/Ensight4Matlab/ensight_lib/lib/libEnsightLib.so.1
You should make sure this file exists. If it doesn't, there has been an error compiling the library. If the file does exist, maybe the paths are not correct.
You can make sure the library has been compiled correctly by building the C++ example first and seeing if it runs, before proceeding to build the MATLAB interface. Let me know if there is any trouble with that.

---

The first warning message tells us that your system has gcc version 5.4.1 installed, which is not supported by MATLAB.
Unfortunately, things get tricky now!
You need to use a gcc version which is supported by MATLAB, since MATLAB ships with a complete C++ std lib runtime. Depending on the MATLAB version you use, this is unfortunately older than the one on your system.

I have tested the following with MATLAB R2016b, which supports gcc 4.9.

First, install a supported version of gcc:

> sudo apt-get install g++-4.9

Now, you have gcc 4.9 installed but the system will still default to gcc 5.
There are two options to proceed, 1) manually changing the system default or 2) changing the build files to point to gcc-4.9, which is less invasive to your system.

To manually change the system default:

> sudo rm /usr/bin/gcc /usr/bin/g++
> sudo ln -s /usr/bin/gcc-4.9 /usr/bin/gcc
> sudo ln -s /usr/bin/g++-4.9 /usr/bin/g++

Then recompile the C++ library (make clean; make in the appropriate folder) and try compiling with MEX again. After you've succeeded, you should restore the original links to your system's default of gcc-5

Alternatively, the following is only slightly more work and doesn't touch the rest of your system:
For the C++ library, edit the Makefile Ensight4Matlab/ensight_lib/Makefile, which has been created by qmake, and change the first lines reading

CC    = gcc
CXX   = g++

to

CC    = gcc-4.9
CXX   = g++-4.9

Then run

> make clean
> make

This should now compile the EnsightLib with a compatible compiler.

Next, on the MATLAB side, edit the file runmex.m. On lines 21-27 it builds a command string for MEX. Here, you need to add the following option to tell it which gcc to use:

' -v GCC= gcc-4.9 '

Make sure to include spaces before and after this, so that the new option is separated from the existing options.
In the output of runmex you should now see gcc-4.9 being used and the warning about an incompatible compiler should disappear.

---

If everything works, the 4 scripts in the examples folder should successfully run now.

[cMadan]

@Kevin-Mattheus-Moerman, have you had a chance to try compiling the code again?

[Kevin-Mattheus-Moerman]

Apologies for the delay! I will resume review this weekend

[Kevin-Mattheus-Moerman]

@schmeisser If I alter runmex.m with:

mex_str = ['mex ',OPTS,... 'CXXFLAGS="\$CXXFLAGS -std=c++11"', ... ' -I',QT_INCLUDE_PATH,' -I',QT_INCLUDE_PATH,'/QtCore',... ' -I',EIGEN_PATH,... ' -I',ENSIGHT_INCLUDE_PATH,... ' -L',QT_LIB_PATH,' -lQtCore -L',ENSIGHT_LIB_PATH,' -lEnsightLib ',... ' -v GCC= gcc-4.9'];

Error using mex
/home/kevin/Desktop/JOSS_review/Ensight4Matlab/EnsightMatlab/gcc-4.9 not found; check that you are in the correct current folder, and check the spelling of '/home/kevin/Desktop/JOSS_review/Ensight4Matlab/EnsightMatlab/gcc-4.9'.

Error in runmex (line 63)
eval([mex_str,' mexfiles/EnsightLib_interface.cpp']);

That made me think I should specify the path to gcc-4.9 so I tried adding '-v GCC= /usr/bin/gcc-4.9 ', but then I get the erro:

Error using mex
Unknown file extension '.9'.

Error in runmex (line 63)
eval([mex_str,' mexfiles/EnsightLib_interface.cpp']);

Am I doing something wrong? Should it be ' -v ''GCC= /usr/bin/gcc-4.9'' '

[Kevin-Mattheus-Moerman]

With that change runmex.m does run it seems, but when I try: newData = EnsightLib; I get:

Warning: The following error was caught while executing 'EnsightLib' class destructor:
Invalid MEX-file '/home/kevin/Desktop/JOSS_review/Ensight4Matlab/EnsightMatlab/EnsightLib_interface.mexa64':
/usr/local/MATLAB/R2016b/bin/glnxa64/../../sys/os/glnxa64/libstdc++.so.6: version `GLIBCXX_3.4.21' not found (required by
/lib/libEnsightLib.so.1).

Error in EnsightLib/delete (line 221)
EnsightLib_interface('interface', 'delete', [1, 0, 5], this.getObjectHandle());

Error in EnsightLib (line 172)
function this = EnsightLib(filename)

In EnsightLib (line 172)
Invalid MEX-file '/home/kevin/Desktop/JOSS_review/Ensight4Matlab/EnsightMatlab/EnsightLib_interface.mexa64':
/usr/local/MATLAB/R2016b/bin/glnxa64/../../sys/os/glnxa64/libstdc++.so.6: version `GLIBCXX_3.4.21' not found (required by
/lib/libEnsightLib.so.1).

Error in EnsightLib (line 180)
this.EnsightObjectHandle = EnsightLib_interface('interface', 'create', valueArray);

[schmeisser]

Sorry, I think there was a typo in my last message.
It should be ' -v GCC=gcc-4.9 ', i.e. there should be no space after (or before) the equals sign.

The second error you see about "version `GLIBCXX_3.4.21' not found" is a symptom of the original problem, compiling with a gcc version and thus a standard library that is newer than the one supported by MATLAB. This should not occur if you fix my typo and try again compiling with gcc 4.9.

[Kevin-Mattheus-Moerman]

I tried again but got the same error message. Then I found this link and used LD_PRELOAD="/usr/lib/x86_64-linux-gnu/libstdc++.so.6" matlab. I was then able to run newData = EnsightLib; without errors. However if I try: jet = EnsightLib('/home/kevin/Desktop/JOSS_review/Ensight4Matlab/data/jet.encas') or jet = EnsightLib('/home/kevin/Desktop/JOSS_review/Ensight4Matlab/data/jet/jet.encas') (after unzipping the data file) I get:

Warning: The following error was caught while executing 'EnsightLib' class destructor:
Error using EnsightLib_interface
Input must be a real uint64 scalar.
Error in EnsightLib/delete (line 221) EnsightLib_interface('interface', 'delete', [1, 0, 5], this.getObjectHandle());
Error in EnsightLib (line 172) function this = EnsightLib(filename)
In EnsightLib (line 172) Error using EnsightLib_interface
Could not read file. EnsightObj::ERROR_STR='[EnsightReader::read()] Cannot read from file
/home/kevin/Desktop/JOSS_review/Ensight4Matlab/data/jet/jet.enc.'
Error in EnsightLib (line 191) [this.EnsightObjectHandle,...

[schmeisser]

The LD_PRELOAD trick sounds like it still used the newer gcc to compile, but you forced Matlab to load a newer library version. As far as I know, that isn't officially supported, but seems to work.

The error message seems to be caused by a truncation bug. As a workaround you can try to cd into the data directory and use a relative path (just jet.encas) instead of an absolute one. In the meantime, I will upload a patch to fix the bug.

[Kevin-Mattheus-Moerman]

Yeah if I cd into the data directory I can run this (note I had to add jet/ as I unzipped the data into the folder jet)
jet = EnsightLib('jet/jet.encas')
I can confirm this works as well as the examples and plot_jet.m.

[Kevin-Mattheus-Moerman]

I'll now spend some more time reviewing the code, examples, documentation, ,and functionality

[Kevin-Mattheus-Moerman]

@whedon commands

[whedon]

Here are some things you can ask me to do:

# List all of Whedon's capabilities
@whedon commands

# Assign a GitHub user as the reviewer of this submission
@whedon assign @username as reviewer

# List the GitHub usernames of the JOSS editors
@whedon list editors

# List of JOSS reviewers together with programming language preferences and domain expertise
@whedon list reviewers

# Change editorial assignment
@whedon assign @username as editor

# Set the software archive DOI at the top of the issue e.g.
@whedon set 10.0000/zenodo.00000 as archive

# Open the review issue
@whedon start review

:construction: Important :construction:

This is all quite new. Please make sure you check the top of the issue after running a @whedon command (you might also need to refresh the page to see the issue update).

[Kevin-Mattheus-Moerman]

@whedon assign @cMadan as editor

[whedon]

OK, the editor is @cMadan

[Kevin-Mattheus-Moerman]

@whedon assign @Kevin-Mattheus-Moerman as reviewer

[whedon]

OK, the reviewer is @Kevin-Mattheus-Moerman

[cMadan]

@Kevin-Mattheus-Moerman - was there a reason we needed to be re-assigned?

[Kevin-Mattheus-Moerman]

@cMadan I'm at the stage where I can reproduce the basic functionality on the MATLAB side of things. I'll formulate some comments shortly on the MATLAB integration and will proceed to tick the boxes above. I think it would be good to also have a reviewer for the C++ elements of this submission. @cMadan do you think it is possible to recruit an additional reviewer?

[Kevin-Mattheus-Moerman]

@cMadan sorry. Above in the top right is said nobody as assigned so I redid it. I think we were both assigned but we did not show up for some reason. @arfon is this a small bug?

[cMadan]

@Kevin-Mattheus-Moerman - I think it doesn't usually list us as 'assigned' on the reviews anymore (as of several months ago), it hasn't seemed to have mattered

Thanks for reviewing the MATLAB portion of the submission! I agree that it would be good to get someone to evaluate the C++ portion. @schmeisser, do you have any suggestions on who could be a good reviewer here? Otherwise I (and @Kevin-Mattheus-Moerman) can try and find one elsewhere

[Kevin-Mattheus-Moerman]

@daviddoria if this submission is of interest to you, would you be able to help review the C++ side of things? I am reviewing the MATLAB integration part.

[daviddoria]

@Kevin-Mattheus-Moerman I'd be happy to look at it, but I'll be travelling through the end of the month. If it can wait until mid-August you can count me in, but if you need it sooner I'll have to pass.

[cMadan]

@daviddoria - that sounds good to me, thank you!

@Kevin-Mattheus-Moerman - are you happy to sign off on the MATLAB portion then, or is there anything else that should be cleared up in the meantime? I know you had some issues with compiling the mex files, so perhaps the installation instructions need to be improved? How about the automated tests and API documentation? Thanks again for all of your work in reviewing this!

[schmeisser]

If there are suggestions on improving the installation instructions regarding the Matlab side of things, I'll be happy to add more information.

[Kevin-Mattheus-Moerman]

@schmeisser I'm in the process of formulating my suggestions. I'm a bit swamped this week but should be able to get to done by Friday

[cMadan]

@Kevin-Mattheus-Moerman - it would be great if you could provide your suggestions soon :)

@daviddoria - are you still available to review the C++ portion of this submission?

[daviddoria]

@cMadan Very sorry for the delay. I will get this done Wednesday evening. First thoughts: WONDERFUL USAGE DOCUMENTATION!! I haven't tried it yet, but I'm very hopeful that it will Just Work™ :)

[schmeisser]

@daviddoria Since the Matlab portion didn't Just Work™, mostly because of building/compiler issues, I'll gladly try to help out if you should run into any problems, and adapt/add to the documentation as necessary.

[daviddoria]

@cMadan @Kevin-Mattheus-Moerman @schmeisser

Hi guys,

Overall, I am very happy with this - nice work! I did a "code review" of the whole repository by forking, creating a branch based off of the first commit in your repository, and submitting a pull request (a cool trick I learned here: http://astrofrog.github.io/blog/2013/04/10/how-to-conduct-a-full-code-review-on-github/)

You can find my comments here:
https://github.com/daviddoria/Ensight4Matlab/pull/1#pullrequestreview-56735748

Most things are pretty minor, but I'll repeat the two "big ticket" items here:

---

1) There does not appear to be a single test - this is very frightening. Even making a trivial sample data set that you read and confirm the contents of would make me feel a little better. But particularly since you're using custom things like the bounding box class, etc. I would never trust it without a bunch of tests.

2) I'd love to see a non-GUI example, basically to demonstrate how easy it is to use the major pieces of the library (the reader and writer). A "5 line" main like:

object = read(file)
do something
write(object, newfile)

would be a welcome addition (and serve as a basic "test" as well (at least an "execution test")).

---

Please let me know if you have any questions, need anything clarified, etc.

Thanks!

David

[cMadan]

Thanks for the detailed review @daviddoria! I really appreciate your thoroughness here (and for the link re: code review).

@schmeisser, let me know after you've had a chance to revise the project based on this feedback.

[schmeisser]

Thanks for the thorough review, I'll get back to you once I've found the time to work through the individual items.

[schmeisser]

Sorry for the long delay, I've been a bit busy.

I've now added the suggested wording about LD_LIBRARY_PATH to the README.md and incorporated the suggestions from your code review, i.e. changed the writer to be in namespace Ensight::Writer, added more documentation, and some other small changes.

The suggested tests are on the way and will be added soon, and I'll also add some minimal C++ examples to go alongside the GUI example, shortly.

[cMadan]

@schmeisser, thanks for the update!

[schmeisser]

I have added some tests in the directory ensight_lib/test and 3 short command line examples in ensight_lib/examples.
For testing that writing works correctly the write_file example can be used, as it does exactly that. It includes reference data files of what the result should be. The output produced by the example can be verified to be correct by using a standard file diff tool to compare it to these reference files.

[daviddoria]

@schmeisser Thanks - we're really close! A few things:

1) I'm not familiar with qmake, but the only way I could get the tests and examples to build was to copy the same Eigen directory into INCLUDE_PATH into both EnsightLibTest.pro and each of the examples .pro files (write_file.pro, etc.). Coming from CMake, that seems super annoying. Is there not a way to tell QMake at the top level "build everything below me" with the same settings? (the equivalent to add_subdirectory() in cmake)?

2) Again not being familiar with QMake I had to poke around a little bit to figure out what happened/what I was supposed to do after I built EnsightLibTest.pro. Obviously, I found the build/ directory and realized I just needed to run the executable EnsightLibTest, but let's add that to the README instructions (especially when the header is "Running tests").

3) The tests did not all pass for me:

********* Finished testing of EnsightConstantTests *********
********* Start testing of EnsightReaderTests *********
Config: Using QTest library 4.8.7, Qt 4.8.7

FAIL!  : EnsightReaderTests::ReadCaseFile_SuccessfulCaseFileRead_CaseFileCorrectlyParsed() 'successfulRead' returned FALSE. ()
   Loc: [ensightreadertests.cpp(79)]

FAIL!  : EnsightReaderTests::QStringRead_SuccessfulCaseFileRead_CaseFileCorrectylParsed() '(bool) createdEnsightObj' returned FALSE. ()
   Loc: [ensightreadertests.cpp(319)]

Totals: 26 passed, 2 failed, 0 skipped

[schmeisser]

1. You're right, I've searched a bit and found a much better way to do this. Two changes: First, there's now a top-level file EnsightLibAndTests.pro that simply builds all other .pro files (library, tests, and examples. Didn't want to make the file name too long, though). If required, the individual pro files can of course still be compiled separately. Second, the INCLUDE_PATH is now defined in file ensight_lib/EnsightLibConfig.pri, which is referenced from all other .pro files, so it needs to be changed only once. This does mean that the path must be absolute now, though, as the referencing .pro files are in different directories.

2. That was quite non-obvious, and also not consistent with the examples. I've simply changed it to put the binary in the same directory, the same way the examples do it. I've also adapted the README to point it out. Hopefully it's clearer now.

3. This is probably because the reference files cannot be found.
   Maybe there's a problems with the symlinks to the file there, although I tested it and they work for me. Otherwise, maybe you just did not unpack the example data in data/jet.tar.bz2. I've added an error message in the tests to point out if files simply cannot be found. I've also changed the README to make this point stick out more.

[daviddoria]

@schmeisser Great, the process went smoothly this time!

A few final things (from me):

1) for paper.md - is this the desired format of the Markdown? https://github.com/ITWM-TVFS/Ensight4Matlab/blob/master/paper/paper.md

I see a "code block" at the top with the author list/affiliation and then the "rendered" Summary and References sections below it.

2) "Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support"

I do not see anything about this in the paper.md

3) "Version: Does the release version given match the GitHub release (v1.0)?"
I don't see any mention of Releases or Versions at all.

David

[Kevin-Mattheus-Moerman]

@daviddoria. In relation to 1), yes that is what the paper looks like at this stage. The authors only need to update the way the references are used (as per my comment below). The paper.md files are automatically rendered into something like this: http://www.theoj.org/joss-papers/joss.00451/10.21105.joss.00451.pdf

[schmeisser]

I'm not sure about these.
I haven't set up any community guidelines, I would be fine if potential contributors contact me directly over github. Is that even possible?
The initial release version would be 1.0, I guess. Where do I have to mention this?

[Kevin-Mattheus-Moerman]

Hi @schmeisser. I can offer some information on community guidelines.

• Github has a useful site on setting up community and contributing guidelines. They also link here for info on contributer guidelines
• You can also check my personal project. The readme there links to a contributing and a conflict of interest document. I recommend adopting something similar.
• Here is a link you can use to base your code of conduct document on

Let me know if you have any other questions.

[Kevin-Mattheus-Moerman]

@schmeisser. About versions. I see you have not minted a release yet in your repository. As part of the JOSS acceptance process you will create a release (the version accepted in JOSS) and archive it for instance using ZENODO (See here to set this up but you can wait until formal acceptance). When you create your first release make sure the version is labeled as v1.0 (completely up to you so make sure you set it correctly).

[Kevin-Mattheus-Moerman]

@schmeisser in relation to your paper. Please make sure your citations are in rMarkdown format.
For instance, instead of writing [3] this citation should be written as: [@gramsch2014simulation], where the keyword after the @ symbol refers to the paper.bib file entries.

Let me know if I can help.

[schmeisser]

@Kevin-Mattheus-Moerman Thanks, I'll check out your links on writing a contribution guideline. I guess for the beginning it's fine to keep it simple.
Can you please check if the updated paper is formatted as required?

[Kevin-Mattheus-Moerman]

@schmeisser Great, yeah you can keep it simple and simple copy past stuff from those suggested ones.
The paper seems appropriately formatted now. If there are any other issues we'll find them at the stage when the PDF is generated.

[schmeisser]

@Kevin-Mattheus-Moerman I have copied some parts of your contribution guideline as you suggested, and also added a section about code of conduct and how to seek support.

[Kevin-Mattheus-Moerman]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

PDF failed to compile for issue #217 with the following error: 

 /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:379:in `parse': (tmp/217/paper/paper.md): did not find expected key while parsing a block mapping at line 2 column 1 (Psych::SyntaxError)
    from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:379:in `parse_stream'
    from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:327:in `parse'
    from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:254:in `load'
    from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:475:in `block in load_file'
    from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:474:in `open'
    from /app/vendor/ruby-2.3.4/lib/ruby/2.3.0/psych.rb:474:in `load_file'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/lib/whedon.rb:73:in `initialize'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/lib/whedon/processor.rb:26:in `new'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/lib/whedon/processor.rb:26:in `set_paper'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/bin/whedon:37:in `prepare'
    from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/command.rb:27:in `run'
    from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/invocation.rb:126:in `invoke_command'
    from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor.rb:387:in `dispatch'
    from /app/vendor/bundle/ruby/2.3.0/gems/thor-0.20.0/lib/thor/base.rb:466:in `start'
    from /app/vendor/bundle/ruby/2.3.0/bundler/gems/whedon-33e05183aa0d/bin/whedon:75:in `<top (required)>'
    from /app/vendor/bundle/ruby/2.3.0/bin/whedon:22:in `load'
    from /app/vendor/bundle/ruby/2.3.0/bin/whedon:22:in `<main>'

[arfon]

@whedon generate pdf

A couple of obvious(?) errors with the paper.md - the title will need quoting in double quotes as there's an apostrophe in the title.

One of the authors is missing an affiliation.

[schmeisser]

@arfon Oops, I fixed that now.

The date in paper.md still says 15 March 2017. Do I need to update this to the current date?

[arfon]

The date in paper.md still says 15 March 2017. Do I need to update this to the current date?

That date doesn't actually show up in the paper anywhere so I think you can leave it as-is.

[arfon]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

https://github.com/openjournals/joss-papers/blob/joss.00217/joss.00217/10.21105.joss.00217.pdf

[Kevin-Mattheus-Moerman]

@arfon I was going to suggest to this author to fix the link to their repo in the paper to be a hyperlink (see last line in paper). But actually I wonder if we should remove this text altogether. I know we ask people to mention the repository link in the paper. But since our PDF article now has a link rendered in it in a clear spot I think there is no need to refer to it in the text anymore. Thoughts?

[arfon]

But since our PDF article now has a link rendered in it in a clear spot I think there is no need to refer to it in the text anymore. Thoughts?

Yes, I've generally been asking authors to remove links like this as we automatically create a prominent link in the JOSS papers.

[schmeisser]

I hope I understood that correctly: I've removed the repository link from paper.md now.

[cMadan]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

https://github.com/openjournals/joss-papers/blob/joss.00217/joss.00217/10.21105.joss.00217.pdf

[arfon]

@cMadan & @Kevin-Mattheus-Moerman - is this good to accept now?

[schmeisser]

@arfon I have made a minor edit to the title and minted a v1.0.0 release.

[cMadan]

@whedon generate pdf

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

https://github.com/openjournals/joss-papers/blob/joss.00217/joss.00217/10.21105.joss.00217.pdf

[Kevin-Mattheus-Moerman]

@cMadan I've just ticked that last box and recommend that we accept this submission.

@daviddoria Do you have any remaining comments?

[daviddoria]

@Kevin-Mattheus-Moerman Nope - all good on my end!

[cMadan]

@arfon, I think we're all set to accept!

@Kevin-Mattheus-Moerman and @daviddoria, thank you both for your thorough reviews of this submission!

[arfon]

@schmeisser - At this point could you make an archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the archive? I can then move forward with accepting the submission.

[Kevin-Mattheus-Moerman]

Looks like its here: from December 7th when they minted v1.0.0

[arfon]

@whedon set 10.5281/zenodo.1095608 as archive

[whedon]

OK. 10.5281/zenodo.1095608 is the archive.

[arfon]

@Kevin-Mattheus-Moerman @schmeisser - thanks for your review here and thanks to @cMadan for editing this submission ✨

@schmeisser - your paper is now accepted into JOSS and your DOI is https://doi.org/10.21105/joss.00217 ⚡️ 🚀 💥

[whedon]

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

If you would like to include a link to your paper from your README use the following code snippet:

[![DOI](http://joss.theoj.org/papers/10.21105/joss.00217/status.svg)](https://doi.org/10.21105/joss.00217)

This is how it will look in your documentation:

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider volunteering to review for us sometime in the future. You can add your name to the reviewer list here: http://joss.theoj.org/reviewer-signup.html