search

lanl / nuDustC

Nucleating Dust Code in C++
BSD 3-Clause "New" or "Revised" License
0 stars 1 forks source link

Installation Documentation #2

Closed steven-murray closed 5 months ago

steven-murray commented 6 months ago

Hi! This issue is a part of my JOSS review here.

Thank you for providing installation instructions, however at least to me they were not super clear. I installed Boost with conda as suggested, however I was expecting the other dependencies to be installed automatically via the cmake (this is partly my own unfamiliarity with cmake, but I expect other users may have this same problem). I found that I needed to install sundials manually, which turned out to be a bit of a hassle (the link to their own installation docs on their readme is a broken link).

For this repo, it would be useful to at least link to the installation docs for sundials when you mention it as a dependency, and also to mention that it must be manually installed when describing the installation instructions. If possible, I'd also give a short default description of how to install sundials, referring to their documentation for more involved use-cases.

As a further note: I find the layout of the sections on the readme a bit confusing. There is a sub-heading "To Build & Run" and then further down a higher-level heading "Building nuDustC++" which is confusing. I think a high-level heading like "Installation" followed by sub-headings "Dependencies" then "Building nuDustC++" would be useful.

guadabsb15 commented 6 months ago

Just adding a couple of thoughts that came to my mind when going through the installation instructions. I agree with @steven-murray comments here, and I think it could be valuable to add in the documentation some extra information. For example, in the case of sundials installation, to enable MPI and OpenMP one has to change ENABLE_MPI and ENABLE_OPENMP to ON. I'm not sure if it is necessary to change other options from their default values, but a note about that would be helpful. In general, it would also be a good idea to mention which version/release of the dependencies you have used or tested. Similarly for compilers and MPI implementations.

sarahstangl-lanl commented 5 months ago

I added and cleaned up the documentation's installation section and added information on enabling OpenMP and MPI. This hopefully is clearer now.