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

Build BOUT++ on RTD #2368

Merged
merged 13 commits into from
Jul 28, 2021
Merged

Build BOUT++ on RTD #2368

merged 13 commits into from
Jul 28, 2021

Conversation

dschwoerer
Copy link
Contributor

No description provided.

@dschwoerer
Copy link
Contributor Author

cmake 3.10 (from ubuntu 18.04, used on rtd) is to old for findPython3 ... maybe worth to implement some fallback (naive backport failed)

@ZedThree
Copy link
Member

ZedThree commented Jul 6, 2021

CMake is available through pip, so could either be installed manually or added to a requirements.txt?

.readthedocs.yaml Outdated Show resolved Hide resolved
This adds documentation for the python interface to the docs.

[skip ci]
[skip ci]
@dschwoerer
Copy link
Contributor Author

Increases the build time from 300..400 to over 600 secs ( 900 is the limit)

I rebased to get rid of all the debug commits.

I also changed the requirements.txt - which isn't strictly necessary - if they shouldn't be changed I can revert.

Thanks @ZedThree for the help 👍

@dschwoerer dschwoerer marked this pull request as ready for review July 6, 2021 12:35
@dschwoerer dschwoerer changed the title WIP: Build BOUT++ on RTD Build BOUT++ on RTD Jul 6, 2021
os.system("which clang-format")
os.system("which clang-format-6.0")
os.system(
"git clone https://github.com/mpark/variant.git ../../externalpackages/mpark.variant"
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing this means RTD doesn't clone the top-level repo so we can't use the submodules?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I forgot the exact failure but git complained it wouldnt know the url for cloning.

ZedThree
ZedThree previously approved these changes Jul 6, 2021
@ZedThree
Copy link
Member

ZedThree commented Jul 6, 2021

Does this also create any documentation for boutcore? I thought it would appear here, but I don't see it: https://bout-dev--2368.org.readthedocs.build/en/2368/api_reference.html

Also, building on RTD does worry me a little bit, partly in terms of the resources used, partly I'm worried it might be fragile, but I'm not sure how else to handle this.

Another way might be to have Github Actions generate the docs and commit them as part of one of the builds? I'm not sure what they actually need to generate, and that still has fun engineering challenges.

@dschwoerer
Copy link
Contributor Author

The documentation is at:
https://bout-dev--2368.org.readthedocs.build/en/2368/user_docs/python_boutcore.html#module-boutcore

You are right, it should be moved to the API section. I noticed it broke the zoidberg docs:
https://bout-dev--2368.org.readthedocs.build/en/2368/_apidoc/zoidberg.html
https://bout-dev.readthedocs.io/en/latest/_apidoc/zoidberg.html

WARNING: autodoc: failed to import module 'zoidberg'; the following exception was raised:
No module named 'scipy'

so I need to add scipy again to the mock models. Further boututils and boutdata should be installed or removed from the docs.

That is generated from the python module, which needs to be loaded at run time. I thought about building the python module in GHA and sharing that with RTD, but that would e.g. require the jobs to be run sequantial, and we would need to have the correct libraries installed on RTD, which I thought would be more stable.

Alternatively I thought about building all the docs on GHA, and only publish them using RTD, but that might not be easier.

Concerning resources, it doesn't even quite double the execution time. I was surprised how fast that was, and we are still below the limit for the free account ...

@ZedThree
Copy link
Member

ZedThree commented Jul 6, 2021

Now that I think about it, compiling all the tests probably more than doubles the compilation time, so I guess it's not too bad.

Let's go with this solution rather than trying to create something more complicated with more moving parts.

Really we need some overarching, joined up solution for the documentation for all the BOUT++ projects, but for now maybe just restore the mocking how it was.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2021

clang-tidy review says "All clean, LGTM! 👍"

[skip ci]
[skip ci]
Also restrict other version, so the dependency resolver doesn't install
dozens of packages to find a matching one.

[skip ci]
@ZedThree
Copy link
Member

I'm going to merge this now, as RTD is currently failing due to the Sphinx/Breathe issues, but the boutcore docs could do with a little polish, e.g. https://bout-dev--2368.org.readthedocs.build/en/2368/_apidoc/boutcore.html#boutcore.PhysicsModel

The boutcore API docs page is also really big without much structure -- I'm not sure what can be done, but that can happen in a future PR

@ZedThree ZedThree merged commit 4b9a253 into next Jul 28, 2021
@ZedThree ZedThree deleted the bc-docs-rtd branch July 28, 2021 10:21
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.

None yet

2 participants