Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update installation instructions #682

Merged
merged 9 commits into from
Mar 16, 2021
Merged

Conversation

rafmudaf
Copy link
Collaborator

@rafmudaf rafmudaf commented Mar 9, 2021

Feature or improvement description
This pull request updates the installation instructions to add clarity and emphasize that compiling is only needed when changing the source code.

Related issue, if one exists
See the many issues where someone is compiling unnecessarily.

Impacted areas of the software
Installation documentation

Additional supporting information
Read this live at https://raf-openfast.readthedocs.io/en/docs-installation/source/install/index.html

Copy link
Contributor

@ebranlard ebranlard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the new binary section with the big warning for the DLLs

I think it would be valuable to start the "build from source" section with a quick start /TDLR.
Something like:

Here's the typical steps for each system, if you encounter problems, or are using a non-standard setup, please read the sections below.

For linux:

apt install xxx
git clone openfast  ; cd openfast
mkdir build ;  cd built  ; cmake ..
./glue-code/openfast/openfast -v

For mac:

brew install xxx
git clone openfast  ; cd openfast
mkdir build ;  cd built  ; cmake ..
./glue-code/openfast/openfast -v

For windows, different alternative are possible. if you have Visual studio, follow the instructions [here].

My favorite on windows is using msys2. Install msys2, open a sys2 shell (I'll probably need to double check):

pacman -Su base-devel git gcc g++ make mingw-w64-x86_64-lapack mingw-w64-x86_64-openblas  

Add the bin folder to you path ROOT_DIR\msys64\mingw64\bin; Then you can do the standard cmake approach

@rafmudaf
Copy link
Collaborator Author

@ebranlard I've added a list of general procedures for compiling. I'm avoiding duplicating information such as dependencies in order to avoid having to update multiple places in the documentation when this changes. I have the full commands for installing dependencies for Mac and linux already. Now in the beginning of the compiling section, there's something like checklist to follow.

@rafmudaf
Copy link
Collaborator Author

As for alternative methods for compiling, it's true that many other methods exist, but I also want to limit the amount of different methods that we support. Ideally, we would have one good pipeline for all systems, and we are actually quite close to that with CMake. In any case, once you have the msys procedure nailed down feel free to add a new section in the documentation describing it. We could add an "Other compiling methods" page or something along those lines. For example, with the new Intel tools (oneAPI) on Windows with CMake, I have to use the NMake Makefile generator rather than the VS generator in order for the Intel compiler to be found. This is a nuance that I would add in an "Others" section.

@ebranlard
Copy link
Contributor

I still think that a linux and mac user would appreciate having all the commands to copy paste in a terminal readily available at the very top, as long as we have a big warning saying, read the documentation if it fails. For windows, it's a bit more tricky and for now we can tell the users to read the whole thing.

@rafmudaf rafmudaf merged commit ccd859d into OpenFAST:dev Mar 16, 2021
@rafmudaf rafmudaf deleted the docs/installation branch March 16, 2021 20:50
@rafmudaf rafmudaf mentioned this pull request May 12, 2021
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants