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

Bump PyTensor dependency #6830

Merged
merged 4 commits into from
Jul 31, 2023
Merged

Conversation

ricardoV94
Copy link
Member

@ricardoV94 ricardoV94 commented Jul 17, 2023

Bumps PyTensor dependency to 2.14.1

This PyTensor release reverts dynamic broadcasting in Elemwise. Code that started to rely on it will fail, hence why it is marked as a major PyMC release as well.

We also keep shape dependencies in the original graph, so some logic that expected eager Constants had to be tweaked.

Finally, no more automatic support shape inference for Multivariate RVs. The thing we had going on wasn't robust and made errors cryptic in the cases where it didn't apply. This includes dropping support for Multivariate CustomDist not implemented with via dist


📚 Documentation preview 📚: https://pymc--6830.org.readthedocs.build/en/6830/

@ricardoV94 ricardoV94 added pytensor major Include in major changes release notes section labels Jul 17, 2023
@codecov
Copy link

codecov bot commented Jul 17, 2023

Codecov Report

Merging #6830 (845c8bb) into main (e3961fc) will decrease coverage by 0.02%.
The diff coverage is 95.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6830      +/-   ##
==========================================
- Coverage   92.05%   92.03%   -0.02%     
==========================================
  Files          96       96              
  Lines       16373    16378       +5     
==========================================
+ Hits        15072    15074       +2     
- Misses       1301     1304       +3     
Files Changed Coverage Δ
pymc/pytensorf.py 92.75% <ø> (ø)
pymc/variational/approximations.py 90.54% <50.00%> (+0.04%) ⬆️
pymc/variational/opvi.py 87.57% <88.23%> (+0.09%) ⬆️
pymc/distributions/distribution.py 96.35% <100.00%> (-0.72%) ⬇️
pymc/distributions/multivariate.py 92.22% <100.00%> (ø)
pymc/gp/cov.py 98.00% <100.00%> (+<0.01%) ⬆️
pymc/logprob/mixture.py 96.92% <100.00%> (+0.02%) ⬆️
pymc/logprob/order.py 97.87% <100.00%> (+0.04%) ⬆️
pymc/math.py 69.15% <100.00%> (-1.47%) ⬇️
pymc/model_graph.py 80.43% <100.00%> (+0.32%) ⬆️
... and 1 more

... and 1 file with indirect coverage changes

@ricardoV94 ricardoV94 force-pushed the bump_major_pytensor_dep branch 2 times, most recently from 2e57d53 to 69927c8 Compare July 17, 2023 13:53
@ricardoV94
Copy link
Member Author

Had to pin arviz from above, as it was installing on the python 3.8 and failing

@ricardoV94 ricardoV94 requested review from twiecki and removed request for OriolAbril July 17, 2023 15:04
@ricardoV94 ricardoV94 changed the title Bump PyTensor dependency to 2.13 Drop support for Python 3.8 Jul 17, 2023
@ricardoV94 ricardoV94 removed the request for review from twiecki July 17, 2023 15:05
setup.py Outdated Show resolved Hide resolved
@twiecki
Copy link
Member

twiecki commented Jul 18, 2023

Let's just drop 3.9 too.

@ricardoV94
Copy link
Member Author

Let's just drop 3.9 too.

3.9 is supported by Numpy until next January. I think Oriol meant 3.8 in the comment he left, if that's what led you to say that?

@OriolAbril
Copy link
Member

I did mean 3.8, sorry

@ricardoV94
Copy link
Member Author

ricardoV94 commented Jul 18, 2023

Last PyTensor release broke something on VI @ferrine.

https://github.com/pymc-devs/pymc/actions/runs/5585372543/jobs/10208039570?pr=6830

My guess it has to do with this one:

But could also be an interaction with this one:

Looking at the step_fn I see one extra NormalRV out of nowhere. And the first evaluation doesn't seem deterministic anymore either, maybe because the replacements are introducing a new RV that didn't use to be introduced (and which is not properly seeded by the logic in compile_pymc)?

Any chance you can take a look?

@ricardoV94 ricardoV94 changed the title Drop support for Python 3.8 Bump PyTensor dependency Jul 18, 2023
@ricardoV94 ricardoV94 force-pushed the bump_major_pytensor_dep branch 3 times, most recently from f8cfe35 to d97d587 Compare July 27, 2023 09:55
@ricardoV94 ricardoV94 requested a review from ferrine July 27, 2023 12:00
@ricardoV94
Copy link
Member Author

Took 3 releases, but CI is passing! :)

@ferrine
Copy link
Member

ferrine commented Jul 28, 2023

I'm good on the vi side, anything else?

@ricardoV94
Copy link
Member Author

Thanks @ferrine! Also with the help debugging!

I requested another review for the non-VI changes. Will wait a bit for feedback before merging.

@michaelosthege
Copy link
Member

major as in PyMC v6 ?

I'm just on the phone until Sunday, so I can't really check the diff. (Don't wait for mel

However, I'd like to request an example of a line of code that will fail and how to fix it to be included in the release notes.

@ricardoV94
Copy link
Member Author

ricardoV94 commented Jul 28, 2023

major as in PyMC v6 ?

No, major as in PyMC v5.X.0

We are not doing "major, major" releases unless the PyMC API / dependencies change drastically.
I don't have a better name for these, so I've been calling them major. Apologies for the confusion

However, I'd like to request an example of a line of code that will fail and how to fix it to be included in the release notes.

We are reintroducing static broadcasting-only again with the PyTensor bump, so for example broadcasting with MutableData("x", np.ones(1, 10)) will fail, unless you write MutableData("x", np.ones(1, 10), shape=(1, None)), or add a specify_shape after the fact.

All other needed changes were internally-only and shouldn't affect users. Mostly, I imagine it will break custom defined MultivariateRVs that relied on the old automatic shape inference..

@ricardoV94 ricardoV94 merged commit 5c600c7 into pymc-devs:main Jul 31, 2023
21 checks passed
@ricardoV94 ricardoV94 deleted the bump_major_pytensor_dep branch September 18, 2023 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major Include in major changes release notes section pytensor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants