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

First version of tutorial material #278

Conversation

mbercx
Copy link
Member

@mbercx mbercx commented Nov 24, 2020

First draft of the tutorial materials! This time in a PR that's a little easier to review 😅.

Questions/TODO

  • Should we add a note on why running takes so long (i.e. the use of a scheduler)?
  • Do we agree that using <PK> and <UUID> everywhere is a good idea in the end?
  • Issue in #4063 from aiida-core still hasn't been fixed. 😅 Assigned myself so I remember to work on it at some point.
  • I've updated the images, but after building with make html, the old ones are still showing... 🤨 Let's see if this also happens here. Seems to be fixed now
  • Do we want to show all the outputs? I have an inclination to say yes for consistency, but perhaps showing the output of e.g. the wget command is a bit much. 🤔

➡️ Remaining questions copied to separate issue #286.

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.

My comments up for discussion

docs/pages/2020_BIGMAP/sections/basics.rst Outdated Show resolved Hide resolved
docs/pages/2020_BIGMAP/sections/basics.rst Outdated Show resolved Hide resolved
docs/pages/2020_BIGMAP/sections/basics.rst Outdated Show resolved Hide resolved
docs/pages/2020_BIGMAP/sections/basics.rst Outdated Show resolved Hide resolved
docs/pages/2020_BIGMAP/sections/qe.rst Outdated Show resolved Hide resolved
docs/pages/2020_BIGMAP/sections/qe.rst Outdated Show resolved Hide resolved
docs/pages/2020_BIGMAP/sections/qe.rst Outdated Show resolved Hide resolved
Co-authored-by: ramirezfranciscof <ramirezfranciscof@users.noreply.github.com>
Copy link
Member Author

@mbercx mbercx left a comment

Choose a reason for hiding this comment

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

Thanks for the review @ramirezfranciscof! Strange that me replying to this review makes GitHub start my own review. Is this because of my newly obtained admin powers? 💪 (EDIT: Nope, I most likely just messed up somehow 😭)

I'll start working on making the necessary changes now.

docs/pages/2020_BIGMAP/sections/basics.rst Outdated Show resolved Hide resolved
docs/pages/2020_BIGMAP/sections/basics.rst Outdated Show resolved Hide resolved
docs/pages/2020_BIGMAP/sections/qe.rst Outdated Show resolved Hide resolved
docs/pages/2020_BIGMAP/sections/qe.rst Outdated Show resolved Hide resolved
docs/pages/2020_BIGMAP/sections/qe.rst Outdated Show resolved Hide resolved
@chrisjsewell
Copy link
Member

chrisjsewell commented Nov 24, 2020

hey @pradyunsg is this right, that pip is downloading multiple versions of the same package 😬 ?

from https://readthedocs.org/projects/aiida-tutorials/builds/12413063/

$ pip install --use-feature 2020-resolver --exists-action=w --no-cache-dir -r requirements.txt

...

Collecting nbformat
  Downloading nbformat-5.0.8-py3-none-any.whl (172 kB)
Collecting nbclient
  Downloading nbclient-0.5.1-py3-none-any.whl (65 kB)
Collecting nbclient
  Downloading nbclient-0.5.0-py3-none-any.whl (65 kB)
Collecting nbformat
  Downloading nbformat-5.0.7-py3-none-any.whl (170 kB)
Collecting nbformat
  Downloading nbformat-5.0.6-py3-none-any.whl (170 kB)
Collecting nbformat
  Downloading nbformat-5.0.5-py3-none-any.whl (170 kB)
Collecting nbformat
  Downloading nbformat-5.0.4-py3-none-any.whl (169 kB)
Collecting nbformat
  Downloading nbformat-5.0.3-py3-none-any.whl (169 kB)
Collecting nbformat
  Downloading nbformat-5.0.2-py3-none-any.whl (169 kB)
Collecting nbformat
  Downloading nbformat-4.4.0-py2.py3-none-any.whl (155 kB)


Command killed due to excessive memory consumption

@mbercx
Copy link
Member Author

mbercx commented Nov 25, 2020

@ramirezfranciscof thanks for the review! I've already given the basics section a good revision, maybe you can have another look at this section?

I'll work on the Quantum ESPRESSO part tomorrow! 😴

@mbercx
Copy link
Member Author

mbercx commented Nov 25, 2020

@ramirezfranciscof I've also reviewed the second part of the tutorial and made more adjustments. So the whole tutorial is once again ready for your review 💪! Also have a look at the questions in the OP.

Note that the RestAPI might still not work (see this comment); give it a go and let me know if you experience similar issues.
➡️ Fixed!

@pradyunsg
Copy link

pradyunsg commented Nov 25, 2020

That backtracking is expected. It would be a good idea to pin any packages that pip's backtracking on, since that's happening due to conflicts somewhere in the graph.

@chrisjsewell
Copy link
Member

That backtracking is expected. It would be a good idea to pin any versions that pip's backtracking on, since that's happening due to conflicts somewhere in the graph.

cheers, I guess the feedback would be that it might be ideal to have the option to not keep these packages in memory (or whatever is happening), but maybe this is just a niche case.

@mbercx so yeh try creating the python environment locally (in a venv) using pip install --use-feature 2020-resolver --exists-action=w --no-cache-dir -r requirements.txt, make sure it actually resolves, then pip list to see what versions you get for the packages like nbclient and nbformat and add them specifically to requirements.txt and/or create a requirements-lock.txt using pip-compile

@mbercx
Copy link
Member Author

mbercx commented Nov 25, 2020

@mbercx so yeh try creating the python environment locally (in a venv) using pip install --use-feature 2020-resolver --exists-action=w --no-cache-dir -r requirements.txt, make sure it actually resolves, then pip list to see what versions you get for the packages like nbclient and nbformat and add them specifically to requirements.txt and/or create a requirements-lock.txt using pip-compile

👌 Thanks @pradyunsg and @chrisjsewell! I'll look into fixing it asap.

@mbercx mbercx force-pushed the tutorial-2020-therealbigmap branch 2 times, most recently from f934663 to 2a2985e Compare November 26, 2020 15:50
@mbercx
Copy link
Member Author

mbercx commented Nov 26, 2020

@chrisjsewell I've updated the requirements.txt, but had to add the nb_render_priority for gettext to the conf.py to make the transifex build work (look here for the error). Besides this, neither the jupyter-download:nb nor the jupyter-download:notebook text roles are recognised for some reason. These should a part of jupyter-sphinx v0.3.2, no?

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.

Second batch of comments. Overall nice work, the tutorial is coming out pretty nice @mbercx !

docs/pages/2020_BIGMAP/sections/basics.rst Outdated Show resolved Hide resolved
docs/pages/2020_BIGMAP/sections/qe.rst Outdated Show resolved Hide resolved
docs/pages/2020_BIGMAP/sections/qe.rst Show resolved Hide resolved
docs/pages/2020_BIGMAP/sections/qe.rst Outdated Show resolved Hide resolved
docs/pages/2020_BIGMAP/sections/qe.rst Outdated Show resolved Hide resolved
Co-authored-by: ramirezfranciscof <ramirezfranciscof@users.noreply.github.com>
@mbercx mbercx merged commit 5dc59ed into aiidateam:tutorial-2020-therealbigmap Nov 26, 2020
@mbercx mbercx deleted the tutorial-2020-therealbigmap branch November 26, 2020 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants