My points for the JOSS review

[nrichart]

Hello,

I am creating an issue to list the thing I noticed while testing the code for the JOSS review openjournals/joss-reviews#7525

I will list the in order of importance from my point of view from most to least important

• The tests documented in the main README do not work for me I opened 2 specific issues Failing Test: Two-particle with wall #35 Failling Test: Compression test #36
• The documentation does not have a Contributing section
• There is no installation part only a build instruction, the code has to be run from the build folder
• The generated documentation does not show properly https://prashjha.github.io/PeriDEM/
  • <a href= and @ref
  • the link for the examples do not work they point to github.io instead of to the source
  • in tools/README.md you mention dependencies that are not in the main README.md like libfann you also install more than the list of dependencies in the brew and apt commands like for example tbb
• The build instruction cannot be copy/pasted due to the comment in the cmake command
• There is no mention that the cmake and make command should/could be run in a build folder

--
Nicolas

[prashjha]

Hi @nrichart,

Thank you for the comments.

The failing tests are due to significant changes. I will rectify them and let you know.

• On installation, I will see if I can modify cmake to include the install option.
• I found it hard to work with links in doxygen. I wanted to display the same files in README.md and doxygen; I could not do this properly with doxygen. I will see if I can correct these errors as well.
• I will address other comments also in due time.

Best,
Prashant

[nrichart]

@prashjha, thanks.
For the doc the main point I think are the section titles that are also links, since this seams to generate 2 errors in the doxygen. It does not show properly but it also mess up the table of content.
The other links I understand your problem, to not have to maintain 2 places with the same content. Could full link help instead of relative ones ?
Best,
Nicolas

[prashjha]

Hello @nrichart,

In the branch 'fix-joss', I have addressed the following:

• Failing two particle test. I replaced the test with the new version in the directory examples/PeriDEM/two_particles/twop_wall_concave_diff_material_diff_size. I tested this and other examples in the example directory, and they all run. Here is the output.log for the examples/PeriDEM/two_particles/twop_wall_concave_diff_material_diff_size example.
• Failing compressive test. The new version is in the directory examples/PeriDEM/compressive_test/compression_large_set. Corresponding output.log is attached.
• I have modified README.md file to not have links in headlines. I hope this will fix some of the issues in generated documentation.

I will keep working on other issue and the issue of proper documentation in coming days.

Best,
Prashant

[prashjha]

Reopened it as there are some unresolved comments.

[prashjha]

Hi @nrichart, I think I have addressed all of your comments with the recent commits. We have some limitations:

1. In the generated documentation, README files in subdirectories mess up the paths to various contents. I will leave it as it is. Our generated documentation allows users to look up the code contents, e.g., functions and classes. In that sense, it serves the purpose.

Alternatively, I may skip rendering README files within subdirectories in the doxygen. Let me know your thoughts.

2. The install currently only installs the executable (I tested it, and it works). It will also be nice to be able to install headers and libraries in the future.

Let me know if this is satisfactory!