Skip to content
This repository has been archived by the owner on Sep 28, 2021. It is now read-only.

Jupyter #19

Merged
merged 59 commits into from
Jul 9, 2020
Merged

Jupyter #19

merged 59 commits into from
Jul 9, 2020

Conversation

felipeZ
Copy link
Member

@felipeZ felipeZ commented May 28, 2020

Create jupyter notebooks for the tutorials:

  • Gromacs QMMM
  • LAMMPS QMMM
  • DFT-GWBSE
  • KMC

@ghost
Copy link

ghost commented May 28, 2020

Congratulations 🎉. DeepCode analyzed your code in 0.365 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard
Edit DeepCode’s Configurations here

@felipeZ
Copy link
Member Author

felipeZ commented Jun 16, 2020

@JensWehner the Notebooks are ready, they take considerable time to run and therefore running then in the CI is not feasible, what shall I do then?

@felipeZ
Copy link
Member Author

felipeZ commented Jul 8, 2020

@JensWehner @junghans How can I know if the Jupyter notebooks are running correctly?

@felipeZ
Copy link
Member Author

felipeZ commented Jul 8, 2020

@JensWehner @junghans How can I know if the Jupyter notebooks are running correctly?

@felipeZ felipeZ closed this Jul 8, 2020
@felipeZ felipeZ reopened this Jul 8, 2020
@felipeZ
Copy link
Member Author

felipeZ commented Jul 8, 2020

I just got confused with the CI. Now I see it

@felipeZ
Copy link
Member Author

felipeZ commented Jul 8, 2020

@junghans can I skip the Sphinx building in ubuntu? It is impossible to generate the documentation from the jupyter notebooks if there is not nbconvert available

# list(APPEND NOTEBOOKS "${PROJECT_SOURCE_DIR}/GROMACS/KMC_Methane/GROMACS_KMC.ipynb")
list(APPEND NOTEBOOKS "${PROJECT_SOURCE_DIR}/LAMMPS/KMC_Thiophene/LAMMPS_KMC.ipynb")

find_package(JUPYTER_NBCONVERT 5.4.1 REQUIRED)
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 the previous solution was better now the build requires nbconvert even to build xtp without any documentation.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have made a revert. The problem is that sphinx needs nbconvert to create the documentation and there is not suitable nbconvert for ubuntu. So, can we skip the Sphinx build on ubuntu

Copy link
Member

Choose a reason for hiding this comment

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

We only have to use it on one single platform. Despite all that, @felipeZ could you add sth. to the developer guide in votca/votca about what the documentation does?

Copy link
Member

Choose a reason for hiding this comment

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

I have made a revert. The problem is that sphinx needs nbconvert to create the documentation and there is not suitable nbconvert for ubuntu. So, can we skip the Sphinx build on ubuntu

Wouldn't

find_package(JUPYTER_NBCONVERT 5.4.1)
if(JUPYTER_NBCONVERT_FOUND AND SPHINX_FOUND)

take care of that?

Copy link
Member

Choose a reason for hiding this comment

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

We only have to use it on one single platform. Despite all that, @felipeZ could you add sth. to the developer guide in votca/votca about what the documentation does?

Correct. We can exclude stuff in the setup action, but I don't want the user to not being able to build xtp on Ubuntu because JUPYTER_NBCONVERT is required. We should try to find a new enough version and if that fails, we just skip that part of the documentation build.

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem is the following, there is a index.rst file in VOTCA/VOTCA that contains a reference to the XTP-TUTORIAL.rst file. When sphinx is trying to build the documentation it will go first to index.rst then to XTP-TUTORIAL.rst. The problem is that inside that last file there are reference to the jupyter notebooks and sphinx will always try to convert those Jupyter notebooks into documentation by running them.
On the other hand, the statement:

find_package(JUPYTER_NBCONVERT 5.4.1)
if(JUPYTER_NBCONVERT_FOUND AND SPHINX_FOUND)

Can just try to run the notebooks or just copy them as they are, but Sphinx will always try to run them anyway and seems there is no nbconvert tool it would fail.
So, I agree with @junghans that the best alternative is just excluding the Sphinx build on Ubuntu.

Copy link
Member

Choose a reason for hiding this comment

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

If we exclude it in the CI it will still trip up user on ubuntu running it manually.

Can we tell Sphinx to ignore the broken links? Or just create a mock version of xtp-tutorials.rst?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can ignore broken links, with nbsphinx_allow_errors = True in the config.py file. I am going to do a PR

@junghans
Copy link
Member

junghans commented Jul 8, 2020

On openSUSE: ModuleNotFoundError: No module named 'pandas'

On Fedora Rawhide: ModuleNotFoundError: No module named 'seaborn'

@junghans
Copy link
Member

junghans commented Jul 9, 2020

On openSUSE:

Notebook error:
PandocMissing in DFTGWBSE_ENERGY.ipynb:
Pandoc wasn't found.
Please check that pandoc is installed:
http://pandoc.org/installing.html

@felipeZ
Copy link
Member Author

felipeZ commented Jul 9, 2020

@junghans @JensWehner It is working!!

@junghans junghans merged commit 5a73bd9 into master Jul 9, 2020
@junghans junghans deleted the jupyter branch July 9, 2020 14:25
votca-bot added a commit to votca/votca that referenced this pull request Jul 9, 2020
@JensWehner
Copy link
Member

@felipeZ good work.

@junghans
Copy link
Member

junghans commented Jul 9, 2020

Fixup in #26

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants