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

updates PyMC requirement #19

Merged
merged 7 commits into from
Dec 25, 2022
Merged

updates PyMC requirement #19

merged 7 commits into from
Dec 25, 2022

Conversation

wiep
Copy link
Contributor

@wiep wiep commented Dec 14, 2022

Set's pymc requirement to >= 5.0.0.

Also, fixes code to configure pymc's/pytensor's default float type to follow PyMC's change of the dependency from aesera to pytensor which is a fork of aesera.

fixes code to configure pymc's/pytensor's default float type
@wiep wiep requested a review from hriebl December 14, 2022 17:37
@wiep wiep added bug Something isn't working enhancement New feature or request labels Dec 16, 2022
@wiep
Copy link
Contributor Author

wiep commented Dec 16, 2022

@hriebl, this PR bumps the jax, jaxlib requirements to 0.4.1.

@jobrachem
Copy link
Contributor

I think it would be nice to make a minor release quite soon, once this PR is merged. Otherwise, the liesel installation from pypi is broken.

@hriebl
Copy link
Contributor

hriebl commented Dec 19, 2022

@jobrachem, why is it currently broken?

@jobrachem
Copy link
Contributor

There's an import error because of (I think) the new Jax version that gets installed alongside Liesel in a fresh venv. Will test this again and post the full error message in the morning, am currently only on the phone.

@hriebl
Copy link
Contributor

hriebl commented Dec 19, 2022

Hmm, I don't seem to get any import errors on my system.

The fact that the tests fail for 9ea3700 seems to be unrelated to this specific commit or PR. It might have something to do with the new NumPy release 1.24 (pip install numpy==1.23.5 partially solves the problem) and the fact that Summary.to_dataframe() returns columns of dtype object in some circumstances. I haven't been able to figure out more, though.

Could any of you investigate this further?

@jobrachem
Copy link
Contributor

There's an import error because of (I think) the new Jax version that gets installed alongside Liesel in a fresh venv. Will test this again and post the full error message in the morning, am currently only on the phone.

I cannot reproduce the issue that I had yesterday in a fresh virtualenv today. Yesterday, I was on an existing virtualenv and my steps, as I recall them, were

  1. pip uninstall liesel,
  2. pip install liesel,
  3. Try to import liesel.model as lsl and import liesel.goose as gs
  4. Observe importerror.
  5. Downgrade jax and jaxlib to the latest 0.3.x versions.
  6. Try again, now it works.

But since I cannot reproduce the issue today, I can only conclude that my message was premature.

@jobrachem
Copy link
Contributor

The fact that the tests fail for 9ea3700 seems to be unrelated to this specific commit or PR. It might have something to do with the new NumPy release 1.24 (pip install numpy==1.23.5 partially solves the problem) and the fact that Summary.to_dataframe() returns columns of dtype object in some circumstances. I haven't been able to figure out more, though.

Could any of you investigate this further?

This is indeed the same error as the one we had in https://github.com/liesel-devs/liesel-internal/issues/200#issuecomment-1358816001. I cannot pin down the issue so far though. The data types in the plot_df columns don't look too suspicious to me:

plot_df.info()
<class 'pandas.core.frame.DataFrame'>
RangeIndex: 30000 entries, 0 to 29999
Data columns (total 6 columns):
 #   Column       Non-Null Count  Dtype   
---  ------       --------------  -----   
 0   param        30000 non-null  object  
 1   param_index  30000 non-null  int64   
 2   chain_index  30000 non-null  category
 3   iteration    30000 non-null  int64   
 4   value        30000 non-null  float32 
 5   param_label  30000 non-null  object  
dtypes: category(1), float32(1), int64(2), object(2)
memory usage: 1.1+ MB

@hriebl
Copy link
Contributor

hriebl commented Dec 20, 2022

Thank you for pointing me to that issue! I had missed it while in Florence.

@hriebl
Copy link
Contributor

hriebl commented Dec 20, 2022

I think I found the reason for the failing tests on this PR:

@hriebl
Copy link
Contributor

hriebl commented Dec 20, 2022

I followed mwaskom/seaborn#3194 and marked NumPy 1.24.0 as incompatible in this PR.

Copy link
Contributor

@hriebl hriebl left a comment

Choose a reason for hiding this comment

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

Now that the tests are working, I'm happy with this PR. Should we go ahead and merge?

@hriebl hriebl marked this pull request as ready for review December 25, 2022 14:57
@hriebl hriebl merged commit 9f1863f into main Dec 25, 2022
@hriebl hriebl deleted the update-dep-pymc branch December 25, 2022 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants