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

Docs: Revise "Get started" flow #4496

Merged

Conversation

csadorf
Copy link
Contributor

@csadorf csadorf commented Oct 21, 2020

This is a comprehensive revision of the "Get started" and "Advanced installation" page to address the issues identified in #4254 .

In summary, the "Get started" page has been completely revised to provide a more linear flow for all supported setup flows and the "Detailed installation" has been largely pruned and renamed to "Advanced configuration" with most content directly incorporated into the linear setup flows.

These are the changes in detail:

  • The "routes overview" panels have been revised to be simpler and more descriptive in their differences.
  • While the "routes overview" panels are still principally distinguished by "level of virtualization", the primary distinction between the first two supported installation methods ("system-wide installation" and "pure conda installation") is the path for installing prerequisite services, not whether the AiiDA Python package is installed into a virtual environment (via conda/venv). The primary difference here is that the first method requires administrative privileges, the latter does not. The revised descriptions should make this much clearer.
  • Each route panel is clickable and leads directly to a full guide for the specific route on a different page.
  • All routes are presented with a linear path, choices (e.g. operating system, Conda/pip) are represented via tabs.
  • Optional steps and potential issues are presented as drop-downs in order to incorporate them into the linear flow without being too distracting to those users where they don't apply.
  • No specific route is clearly recommend over another because it really depends on the user environment and use case. However the system-wide installation is likely most applicable to novice users who – given the presented options – are unsure about which route is best for them. This is based on the assumption that the alternative routes would likely not be applicable to those users who are unfamiliar with the mentioned technology in the first place. For example, a user should probably not take the docker route if they are unfamiliar with docker, however users who are familiar with docker, will likely be able to evaluate whether that is the right choice for them. The system-wide installation is therefore now explicitly recommended to those users who are unsure about which route is best for them.
  • The "Detailed Installation" page has been renamed to "Advanced configuration". This is motivated by the fact that all supported setup routes are now fully covered on the "Getting started" page. Caveats and potential issues are by and large directly incorporated into those linear paths. The information found on the "Advanced configuration" page now covers edge cases for advanced users who want to further customize their setup or who have rare environmental conditions. The fact that the content of that chapter will likely not be applicable to most users is made explicit at the top of the page.
  • Slight duplication between different routes is deliberately tolerated to be able to represent a linear flow to users.
  • The instructions for the Windows Subsystem for Linux have been updated and now support both version 1 and 2.

@csadorf csadorf force-pushed the docs/issue-4254-installation-flow branch from 3185544 to 7d11104 Compare October 21, 2020 08:33
@csadorf csadorf changed the title Docs/issue 4254 installation flow Docs: Revise "Get started" flow Oct 21, 2020
@chrisjsewell
Copy link
Member

Ah look at those awesome tabs! I wonder who wrote that code 😝

https://1535-77234579-gh.circle-artifacts.com/0/html/intro/get_started.html

@csadorf
Copy link
Contributor Author

csadorf commented Oct 21, 2020

@greschd and @zhubonan Could you help me fix up the Windows Subsystem installation flow? I don't think it is 100% correct/complete at the moment. Unfortunately I do not have access to a windows machine.
https://1538-77234579-gh.circle-artifacts.com/0/html/intro/get_started.html#intro-get-started-system-wide-install

@csadorf
Copy link
Contributor Author

csadorf commented Oct 21, 2020

Ah look at those awesome tabs! I wonder who wrote that code 😝

https://1535-77234579-gh.circle-artifacts.com/0/html/intro/get_started.html

The tabs are really great! 👍 👍

Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

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

initial comments


*Use a pre-made image of all the required software.*
.. link-button:: intro:get_started:virtual-machine
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps link straight to https://quantum-mobile.readthedocs.io here

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 think that's a good idea. Do you know if it is possible to open the link in a new tab/window?

docs/source/intro/get_started.rst Outdated Show resolved Hide resolved
docs/source/intro/get_started.rst Outdated Show resolved Hide resolved

.. tabbed:: Ubuntu

*AiiDA is tested on Ubuntu versions 14.04, 16.04, and 18.04.*
Copy link
Member

Choose a reason for hiding this comment

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

We may want to update this at some point to 16.04, 18.04, 20.04.
I've anyhow started upgrading Quantum Mobile roles to be tested against 18.04 and 20.04

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 will just update that now.

Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

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

that was meant to be request changes

@greschd
Copy link
Member

greschd commented Oct 21, 2020

@greschd and @zhubonan Could you help me fix up the Windows Subsystem installation flow? I don't think it is 100% correct/complete at the moment. Unfortunately I do not have access to a windows machine.
https://1538-77234579-gh.circle-artifacts.com/0/html/intro/get_started.html#intro-get-started-system-wide-install

I use the Ubuntu rabbitmq now (I think this didn't work in WSL1) - not sure if we should change the instructions accordingly. @zhubonan what's your experience here?
If we switch to Ubuntu rabbitmq, the startup script needs to include sudo service rabbitmq-server start.

Two other points:

  • With the visudo'd file, we should also make sure the instructions are such that the file is owned by and only editable by root. If any user can edit the file, this is a privilege escalation.
  • We can add instructions for executing that file on startup with Task Scheduler.

@ramirezfranciscof
Copy link
Member

Paging @ltalirz and @sphuber who probably also are interested in checking how this turned out. Should @csadorf maybe give a general overview/demo of how it looks like today in the AiiDA meeting so that @giovannipizzi can take a look at it without having to go through the PR now?

@csadorf
Copy link
Contributor Author

csadorf commented Oct 21, 2020

@ramirezfranciscof I've marked this PR as a draft, because the WSL instructions still need to be fixed.

@csadorf csadorf linked an issue Oct 21, 2020 that may be closed by this pull request
@csadorf
Copy link
Contributor Author

csadorf commented Oct 21, 2020

I use the Ubuntu rabbitmq now (I think this didn't work in WSL1) - not sure if we should change the instructions accordingly. @zhubonan what's your experience here?
If we switch to Ubuntu rabbitmq, the startup script needs to include sudo service rabbitmq-server start.

Two other points:

  • With the visudo'd file, we should also make sure the instructions are such that the file is owned by and only editable by root. If any user can edit the file, this is a privilege escalation.

  • We can add instructions for executing that file on startup with Task Scheduler.

@greschd Thank you very much, that is already very helpful. Would you potentially be able/willing to directly push an edit to this branch?

@csadorf csadorf requested a review from chrisjsewell October 21, 2020 16:35
@zhubonan
Copy link
Contributor

@csadorf @greschd I think on WSL1 the Ubuntu native RabbitMQ does not work. We should definitely differentiate between WSL1 and WSL2. The current instructions seem OK to me for WSL1.

I have not used WSL2 @greschd. My main concern is that the files system access to the Windows side appears to be much slower in WSL2 than WSL1 and I store my data in the Windows file system. Also, WSL2 seems to block quite a bit RAM? I have WSL2 installed in a different machine but I don't have AiiDA installed there. How did you find the WSL2 experience so far? Maybe there should be some remarks about choosing between the two,

@greschd
Copy link
Member

greschd commented Oct 21, 2020

Yeah I've had mostly good experiences with WSL2 - but the files I work on with WSL aren't on the Windows drive. Not sure about the RAM issue, I would probably need to look more closely. But this is just my development environment, production is on a separate machine.

The only difference between WSL1/2 in terms of installing would be where RabbitMQ is installed. The "startup via TaskScheduler" would again be the same.

@zhubonan
Copy link
Contributor

@greschd I see. For development it should be pretty good, I run both production and development in WSL so disk space is a concern. Also, the growing VHD files for WSL2 will be very big if used for production I would imaging, and that is not easily reclaimable.
@csadorf I have a feeling that we should warn about potential problems of WSL2 (e.g. disk space) for production?

@csadorf
Copy link
Contributor Author

csadorf commented Oct 22, 2020

@zhubonan It seems to me like we should show instructions for WSL 2, but show a disclaimer that there might be issues with file system performance in which case we recommend that users should use version 1 (and mention the difference w.r.t. RabbitMQ). Beyond that I think we should be careful about taking on too much responsibility about shortcomings of various systems that are outside of our control.

@greschd Could you help me with the instructions for the scheduler? Do users still need to create the mentioned script and then place it in the TaskScheduler or so?

Both, can you verify that the mentioned issue concerning time zones is still a problem? Reading through the issue comments it might be resolved?

@greschd
Copy link
Member

greschd commented Oct 22, 2020

Yes, the TaskScheduler would just execute that script at startup; I can write some instructions, but not sure how fool-proof that will be.

I never had the timezone issue AFAICR.

@csadorf
Copy link
Contributor Author

csadorf commented Oct 22, 2020

Yes, the TaskScheduler would just execute that script at startup; I can write some instructions, but not sure how fool-proof that will be.

That would be very much appreciated. I will try to find some additional testers as well.

I never had the timezone issue AFAICR.

Ok, I'm inclined to remove that.

.. _intro:install:docker:

****************************
Run AiiDA via a Docker image
Copy link
Member

Choose a reason for hiding this comment

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

This removes a number of helpful commands from the original "Using the Docker image" section, for example the one you need to actual enter the container:

docker exec -it --user aiida aiida-container /bin/bash

plus the note about using /tmp/my_data, which is the only place writable by the aiida user, and the command to mount it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was actually a bit confused about that section. I'll restore the things that got removed, but I also want to avoid making this too much of a general docker tutorial. But I'm slightly less concerned about the length now that the instructions are on their own separate page.

@greschd greschd force-pushed the docs/issue-4254-installation-flow branch from 925e1a2 to ad029a1 Compare October 22, 2020 10:28
@greschd
Copy link
Member

greschd commented Oct 22, 2020

@csadorf I added the text but didn't check the formatting - my local install is slightly broken ATM (at least, it can't build the docs). Could you format it to your liking?
Feel free to also rephrase it however you see fit - I didn't spend a ton of time on that.

@greschd greschd force-pushed the docs/issue-4254-installation-flow branch from 35e0131 to ccae4fe Compare October 22, 2020 12:05
@csadorf
Copy link
Contributor Author

csadorf commented Oct 22, 2020

@csadorf I added the text but didn't check the formatting - my local install is slightly broken ATM (at least, it can't build the docs). Could you format it to your liking?
Feel free to also rephrase it however you see fit - I didn't spend a ton of time on that.

Great, very much appreciated. No worries about the formatting, I will take care of that.

greschd
greschd previously approved these changes Nov 5, 2020
Copy link
Member

@greschd greschd left a comment

Choose a reason for hiding this comment

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

The WSL instructions look good to me, except I'm not 100% sure about the instructions I wrote anymore (see comment) 😅

@csadorf
Copy link
Contributor Author

csadorf commented Nov 5, 2020

Thanks a lot @csadorf . This looks really good, excellent work. I have not been too closely involved in this part of the docs, so I will abstain from a full judgement, but from looking at the built documentation roughly, it looks good. This would be fine for me to merge, but I let the approving up to someone else.

One comment: sometimes a "Hint" box is used where I think "Tip" may be better. For example, on the system-wide installation page the "hint" "Do not start more daemons then there are physical processors on your system." seems more like a tip. Some goes for some of the others. Not sure if there is no "Tip" box or if this decision was deliberate. Anyway, it is a tiny comment and not very important

Thanks a lot! I am actually not 100% clear on the difference between "hint" and "tip". Maybe that's something we should clarify in the wiki? I'd be fine with just using "tip" everywhere instead of "hint".

csadorf and others added 13 commits November 5, 2020 13:54
 - Overhaul of page structure and installation flow.
 - Incorporate content from detailed installation page.
And remove all content covered by the get started page.
That makes it more likely for users to choose that method which is
overall still better supported than Conda; the latter has no provision
to install extras.
- Distinguishes between WSL 1 / 2 by how RabbitMQ is installed only
- Adds instructions for setting up a task in Task Scheduler for
  starting the services.
- Removes the warning about the timezone problem, as it seems to
  be fixed.
To retain more from the previous advanced installation instructions.
@csadorf csadorf force-pushed the docs/issue-4254-installation-flow branch from 1774ae8 to a9c57c6 Compare November 5, 2020 12:54
@csadorf csadorf requested a review from sphuber November 5, 2020 12:54
@csadorf
Copy link
Contributor Author

csadorf commented Nov 5, 2020

@greschd @chrisjsewell Sorry, your reviews were dismissed when I replaced all 'hints' with 'tips'. This should have been the last commit on this.

@csadorf csadorf requested a review from greschd November 5, 2020 12:55
@csadorf
Copy link
Contributor Author

csadorf commented Nov 6, 2020

@ramirezfranciscof Do you still want to review this or should I merge it?

@ramirezfranciscof
Copy link
Member

I was going to give it a pass today but got other inconvenients I had to (and still have to) solve. Since I think you already received enough reviews from other people I think it is ok if you want to go ahead and merge it.

@csadorf
Copy link
Contributor Author

csadorf commented Nov 6, 2020

I was going to give it a pass today but got other inconvenients I had to (and still have to) solve. Since I think you already received enough reviews from other people I think it is ok if you want to go ahead and merge it.

Ok, I'll merge it then.

Copy link
Member

@ramirezfranciscof ramirezfranciscof left a comment

Choose a reason for hiding this comment

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

Pass given, this looks great!

My only tiny silly comment would be if we could add somewhere in the System-wide tile that this is the recommended installation for a work setup. It might be obvious from the text in the tiles that if you don't have admin then you should do the conda-env, but perhaps if you do have it, you may be unsure of wether you should be using it or not (kind of like you shouldn't use "sudo pip install" even if you are sudoer).

Anyway, up to you.

@csadorf
Copy link
Contributor Author

csadorf commented Nov 6, 2020

Pass given, this looks great!

My only tiny silly comment would be if we could add somewhere in the System-wide tile that this is the recommended installation for a work setup. It might be obvious from the text in the tiles that if you don't have admin then you should do the conda-env, but perhaps if you do have it, you may be unsure of wether you should be using it or not (kind of like you shouldn't use "sudo pip install" even if you are sudoer).

Anyway, up to you.

Like I said in the description of this PR, I don't think it is that easy to recommend one method over the other. It depends on your circumstances, your needs, and your familiarity with certain technologies. That the system-wide method requires admin rights, and the pure-conda method does not should be abundantly clear now. Therefore, in the proposed revision we do not recommend any specific method over another, but instead show a slight preference for specific methods via the ordering of the tiles, and furthermore, via this sentence:

If you are unsure, use the system-wide installation method.

@ramirezfranciscof
Copy link
Member

Ah, sorry, I totally missed that sentence. Disregard the comment, all GTG.

@ramirezfranciscof ramirezfranciscof merged commit 5c12056 into aiidateam:develop Nov 6, 2020
@csadorf csadorf deleted the docs/issue-4254-installation-flow branch November 6, 2020 17:54
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.

Docs: flow of Introduction section unclear
6 participants