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 numpyro install to building docs instructions #5936

Merged
merged 1 commit into from
Jul 2, 2022
Merged

Add numpyro install to building docs instructions #5936

merged 1 commit into from
Jul 2, 2022

Conversation

isms
Copy link
Contributor

@isms isms commented Jun 30, 2022

Fixes #5935

@twiecki
Copy link
Member

twiecki commented Jul 1, 2022

We might want to consider adding this to requirements-dev.txt.

@isms
Copy link
Contributor Author

isms commented Jul 1, 2022

Yes, unless there is a compelling interest to keeping this separate, in which case a requirements-jax.txt or something could be a middle ground. That would centralize the prereqs (with pinned versions if necessary) so it does not have to be explained in multiple places but can still be skipped in, e.g., environments that must run tests but would have a problem installing JAX.

@OriolAbril
Copy link
Member

The readthedocs check passed and has always passed installing only the dependencies in https://github.com/pymc-devs/pymc/blob/main/conda-envs/environment-dev.yml (should be the same as in requirements-dev.txt). I think the error you mention in thr issue is different. One reason might be differemces between the conda and txt requirements, maybe jax is missing from the txt? jax is needed to import anything in sampling jax file, but numpyro and blackjax are both optional

@isms
Copy link
Contributor Author

isms commented Jul 1, 2022

One reason might be differemces between the conda and txt requirements, maybe jax is missing from the txt? jax is needed to import anything in sampling jax file, but numpyro and blackjax are both optional

So jax is definitely missing from requirements-dev.txt, but I assumed that was done on purpose because it is specifically excluded in scripts/generate_pip_deps_from_conda.py (introduced via #5477).

Edit to add, the squashed commit has some possible rationale:

* Add jax to dev environment

* Jax doesn't work on windows, remove it

* Exclude jax

@michaelosthege
Copy link
Member

I think we should continue to exclude jax dependencies from requirements-dev.txt until they are available on all platforms.

But why are you just adding numpyro to the instructions and not blackjax?

@michaelosthege
Copy link
Member

Update, @isms: in #5938 I propose to remove the docs build script for Windows, and also added a note in the paragraph above the code block you're changing.

I'd just like to understand if we should also include blackjax here?

@isms
Copy link
Contributor Author

isms commented Jul 2, 2022

@michaelosthege Installing blackjax is not necessary to make the documentation build once numpyro is installed, that's the only reason not to include both. Happy to add it back if that feels important.

@michaelosthege michaelosthege merged commit d9197ef into pymc-devs:main Jul 2, 2022
@michaelosthege
Copy link
Member

Thanks @isms !

@isms isms deleted the 5935-add-numpyro-to-building-docs branch July 2, 2022 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Instructions on building docs do not mention need to install numpyro
4 participants