-
-
Notifications
You must be signed in to change notification settings - Fork 12
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 test to check for warnings #41
Conversation
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
@conda-forge-admin, please rerender |
Hi! This is the friendly automated conda-forge-webservice. I tried to rerender for you, but it looks like there was nothing to do. This message was generated by GitHub actions workflow run https://github.com/conda-forge/pymc-feedstock/actions/runs/2440243349. |
xref: #37 (comment) |
My test already bears fruit! Under Windows:
Do we want Windows users to have a different BLAS implementation? Or do we want to allow this warning? |
I tried the install command from our Wiki page which is currently this:
The import gives me no
My
I believe this is good, because it means I actually got the faster backend through some of the extra dependencies from the install command? But the other print statements look scary. This is the corresponding ticket: conda-forge/aesara-feedstock#54 |
That's interesting. What happens if you do For reference, here are Aesera's BLAS instructions Since Then I'm wondering what is the purpose of each of the three additional libraries: |
It would be really nice if we could get the install process as simple, streamlined, and reliable as possible! 😄 Of course there will always be tradeoffs between speed and disk usage and licensing, but it feels rather confusing right now how to make an informed decision. (But I may be missing some docs...) |
Some time ago I tried a lot of different combinations and found that adding This is the result of creating an env with
I'll try some more combinations, hang on |
Do you have a |
No I don't have a
...
|
Cool, so installing That is a very large package on Windows, right? So probably it makes sense to leave this as "optional but recommended", right? |
Then it is somewhat mysterious why you have a warning about those missing executables but it doesn't show on the GitHub CI 🤔 |
I wouldn't worry about the size. |
Shall we make BTW, I just looked more closely into the CI logs:
Ah!!! I'm not parsing that because I'm looking for |
Yes, I think we should include I added both |
I'm thinking we should change the recipe to define two outputs: pymc-base and pymc, where pymc-base omits even the compilers and pymc is the recommended batteries-included config. |
We used to have mkl here. But adding mkl here would prevent Apple M1 chips
from working. It's also slightly in bad style to fix the blas
implementation someone might want to use.
I do like the idea of separate packages though.
…On Sun, Jun 5, 2022, 10:33 Ben Mares ***@***.***> wrote:
I'm thinking we should change the recipe to define two outputs: pymc-base
and pymc, where pymc-base omits even the compilers and pymc is the
recommended batteries-included config.
—
Reply to this email directly, view it on GitHub
<#41 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAFETGDXVYUJOXRRKC4KZA3VNRQ5BANCNFSM5X3WBSFA>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
I think I managed to set the selector correctly. We should probably render the builds as artifacts so we can download them in order to test the M1 builds. I'm not sure what's going on, but for some reason the tests run only for
I'll have to look into it on another day. |
@maresb This looks great! Can we still merge? We plan to announce tomorrow and having a |
@twiecki my tests for pymc were not running, so much more likely than not things are broken. 🙁 |
@maresb I tried it on osx and it worked. As long as the right packages are getting installed, things should be fine, no? |
I also noticed that |
Osx M1 or Intel? Pymc = pymc-base + compilers, blas, etc. |
M1 |
To be more precise, pymc-base is the python package plus the python dependencies, and nothing more. Great that M1 is working! |
I can confirm that on Linux I can import Anyone capable of testing Windows? @michaelosthege? It would be nice to also test inference for some extremely basic model here on Conda-Forge, just to make sure that that doesn't trigger any warnings either. |
I'm still very confused why the tests aren't running for the |
I can confirm that the BLAS warning is now gone on Windows. This leaves us with the other warnings which are an Aesara issue. |
Hi, I've just tested install PyMC v4 on Ubuntu 20.04 The installation using this
|
After specify exact version of PyMC 4.0.0, it can collect the right versions:
However, after the installation, the
|
@danhphan Very odd, can you open a new issue? |
Hi @danhphan, I have a few suggestions for you... It looks like you are installing the latest
When I do this on Ubuntu 20.04 I get the correct packages and no warnings. In case you want to choose your own BLAS, then you should instead install You will have much better performance when solving the environment if you use Mamba, which is a drop-in replacement for
Finally, to ensure that you're installing the best version, on the command line you can specify
where the quotes are necessary because Bash would otherwise interpret the |
Sorry, I should have also included the previous command. It was a new conda environment, and I use this conda command:
|
Hi, confirm that using this command
|
The is no So I am not sure why there is an above issue if we create a (new) conda environment first, then install the
|
@danhphan, thanks for the clear instructions, I can reproduce the issue. I can fix the problem by replacing I don't have time at the moment to look into this more deeply. I'll try to find some time later tonight. But since this is confirmed, could you please open a new issue? |
Hi thanks, I have opened a new issue at #43 Not sure if this is the right way to run
However, there is still the
|
I guess MKL is not getting installed? |
Yes, when using
So the mkl is there, but I'm not sure why there is still Result when runnning
|
Solution proposed in #43 |
Closes #40 by bumping build number
Adds a post-build test to ensure that
pymc
imports without warnings (e.g. compiler not found).Attn: @michaelosthege
Checklist
0
(if the version changed)conda-smithy
(Use the phrase@conda-forge-admin, please rerender
in a comment in this PR for automated rerendering)