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

Add CNS installation instructions #469

Merged
merged 18 commits into from
Nov 22, 2022
Merged

Conversation

cvnoort
Copy link
Contributor

@cvnoort cvnoort commented Jun 7, 2022

General instructions for installing/recompiling CNS for HADDOCK added to INSTALL.md.

You are about to submit a new Pull Request. Before continuing make sure you read the contributing guidelines and you comply with the following criteria:

  • You have stick to Python. Talk with us before adding other programming languages to HADDOCK3
  • Your PR is about CNS
  • Your code is well documented: proper docstrings and explanatory comments for those tricky parts
  • You structured the code into small functions as much as possible. You can use classes if there's a (state) purpose
  • code follows our coding style
  • You wrote tests for the new code
  • tox tests pass. Run tox command inside the repository folder
  • -test.cfg examples execute without errors. Inside examples/ run python run_tests.py -b
  • PR does not add any install dependencies unless permission granted by the HADDOCK team
  • PR does not break licensing
  • Your PR is about writing documentation for already existing code 🔥
  • Your PR is about writing tests for already existing code :godmode:

Added section to INSTALL.md that lists the basic steps for installing CNS.

For now, it links to the CNS routines to add on Dropbox. I would be happy to also add these to the repository and update the instructions accordingly, would just need to know where.

General instructions for installing/recompiling CNS for HADDOCK added to INSTALL.md.
@amjjbonvin
Copy link
Member

I would suggest to create a separate document just for CNS and refer to it in the INSTALLATION.md file

Copy link
Member

@amjjbonvin amjjbonvin left a comment

Choose a reason for hiding this comment

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

This is a rather simple description. We need to extend to put some compiler requirements if needed and also make a separate document just for CNS.

docs/INSTALL.md Outdated

#### Add HADDOCK routines

HADDOCK requires some adjustments in CNS to function properly. Download the [set of routines to add](https://www.dropbox.com/s/wliubqovuusqdvr/cns.tgz?dl=0). Uncompress the archive and move all of its files into the CNS source directory (`PATH/TO/cns_solve_1.3/source/`).
Copy link
Member

Choose a reason for hiding this comment

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

Where is the code coming from? Is this something we are providing? If not, may-be we better create our own archive and provide it under SURF drive for example (I can handle that)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a link that was provided to a user under #329 , but can of course be replaced with anything more appropriate.

Copy link
Member

@rvhonorato rvhonorato Jun 7, 2022

Choose a reason for hiding this comment

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

Is this the same as haddock2.4/cns1.3? Since these might be crucial for the proper work of cns and haddock3, would it not be better if we package it together with the repo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would be happy to also add these to the repository and update the instructions accordingly, would just need to know where.
Added a link to the CNS instructions at the beginning of INSTALL.md, but if/when we add these files to the haddock3 repository it should probably be moved down to a step after cloning.

docs/INSTALL.md Outdated

HADDOCK requires some adjustments in CNS to function properly. Download the [set of routines to add](https://www.dropbox.com/s/wliubqovuusqdvr/cns.tgz?dl=0). Uncompress the archive and move all of its files into the CNS source directory (`PATH/TO/cns_solve_1.3/source/`).

#### Recompile CNS
Copy link
Member

Choose a reason for hiding this comment

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

What about the compiler requirements? And any OS-specific instructions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will add more details once I've moved the CNS instructions to a separate file!

@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2022

Codecov Report

Merging #469 (00e0e78) into main (3258755) will not change coverage.
The diff coverage is n/a.

❗ Current head 00e0e78 differs from pull request most recent head af5e6e0. Consider uploading reports for the commit af5e6e0 to get more accurate results

@@           Coverage Diff           @@
##             main     #469   +/-   ##
=======================================
  Coverage   72.41%   72.41%           
=======================================
  Files          95       95           
  Lines        6124     6124           
=======================================
  Hits         4435     4435           
  Misses       1689     1689           
Impacted Files Coverage Δ
tests/test_module_topoaa.py 70.68% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3258755...af5e6e0. Read the comment docs.

@amjjbonvin
Copy link
Member

amjjbonvin commented Jun 8, 2022 via email

@rvhonorato rvhonorato added documentation Improve docs CNS Improvements in the CNS engine labels Jun 8, 2022
Moving CNS installation instructions from INSTALL.md to its own file (CNS.md).
Expanded information on the compilation of CNS. More details to be added particularly about the Makefile and different compilers/systems.
Updated to link directly to the "Installation" page of the CNS website, rather than to the main menu.
Some information on Makefile headers with specific details for gfortran.
Replacing the link path for mac to show an example of a self-compiled executable instead of the precompiled UU-specific one. For both mac and linux, specify the name of the CNS_FOLDER (cns_solve_1.3).
Copy link
Member

@joaomcteixeira joaomcteixeira left a comment

Choose a reason for hiding this comment

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

Great work @cvnoort! Left some some comments on the pertinent lines 👍

docs/CNS.md Outdated
source .cns_solve_env_sh
```

To avoid having to repeat this step each time you open a terminal window to run CNS/HADDOCK, you may want to add the relevant `source` line to your `~/.cshrc`, `~/.tcshrc`, `~/.bashrc`, or `~/.bash_profile`. When doing this, make sure to use the full path instead of only the file name.
Copy link
Member

Choose a reason for hiding this comment

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

I think this is not needed to run HADDOCK. At least, I don't have it in my .bashrc. It doesn't hurt, though ☺️


# on linux
ln -s /PATH/TO/CNS_FOLDER/intel-x86_64bit-linux/source/cns_solve-2002171359.exe bin/cns
ln -s /PATH/TO/cns_solve_1.3/intel-x86_64bit-linux/source/cns_solve-2002171359.exe bin/cns
Copy link
Member

Choose a reason for hiding this comment

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

I suggest keeping the CNS_FOLDER instead of cns_solve_1.3 because it better explains the command, as it is more general.

docs/CNS.md Outdated

## 3 Compile CNS

#### Start compilation
Copy link
Member

Choose a reason for hiding this comment

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

This head should have 3 # instead of 4.

### Start compilation

docs/CNS.md Outdated

If this worked, you can continue to the [next step](#4-Source-CNS).

#### Install a compiler
Copy link
Member

Choose a reason for hiding this comment

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

I suggest moving this section and the next one to a Troubleshooting section at the end of the file. Like that, we can continue adding examples and solutions to possible future problems without perturbing the reading flow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This sounds great to me, I was thinking something similar to prevent confusing users with too much information when everything runs smoothly for them.

Moving some compilation details to a separate Troubleshooting section at the bottom of the file.
Brief statement about importance of CNS and extra link to INSTALL.md to make sure repository (eventually with the additional CNS routines) is cloned before CNS installation.
Update of the CNS instructions to point to the custom CNS routines that are now provided in the haddock3 repository, instead of a Dropbox link.
Point users to the CNS installation instructions after cloning the haddock3 repository.
@cvnoort cvnoort marked this pull request as ready for review November 17, 2022 15:54
@amjjbonvin amjjbonvin merged commit d0bca1c into haddocking:main Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CNS Improvements in the CNS engine documentation Improve docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants