Updates for Theano-PyMC 1.1.0

[brandonwillard]

This PR provides compatibility updates for Theano-PyMC 1.0.15.

[codecov]

Codecov Report

Merging #4405 (bcd447d) into master (1769258) will increase coverage by 0.14%.
The diff coverage is 92.30%.

@@            Coverage Diff             @@
##           master    #4405      +/-   ##
==========================================
+ Coverage   88.00%   88.15%   +0.14%     
==========================================
  Files          88       88              
  Lines       14342    14571     +229     
==========================================
+ Hits        12622    12845     +223     
- Misses       1720     1726       +6     

Impacted Files	Coverage Δ
pymc3/math.py	68.81% <60.00%> (+0.16%)	⬆️
pymc3/data.py	80.35% <100.00%> (+0.17%)	⬆️
pymc3/distributions/dist_math.py	92.22% <100.00%> (-3.85%)	⬇️
pymc3/distributions/multivariate.py	83.42% <100.00%> (-0.13%)	⬇️
pymc3/model.py	89.30% <100.00%> (+0.39%)	⬆️
pymc3/model_graph.py	88.57% <100.00%> (ø)
pymc3/ode/ode.py	93.87% <100.00%> (+0.06%)	⬆️
pymc3/theanof.py	90.56% <100.00%> (+0.08%)	⬆️
pymc3/distributions/bart.py	80.15% <0.00%> (-0.65%)	⬇️
... and 42 more

[michaelosthege]

This is still WIP. Whoever wants to help please do.
Here is what I diagnosed so far:

• pymc3.tests.test_distributions_random.TestTruncatedNormal.test_scalar_distribution_shape → shape error
• pymc3/tests/test_distributions_random.py:284: module 'pymc3.theanof' has no attribute 'change_flags'
• pymc3/tests/test_distributions.py::TestMatchesScipy::test_weibull → has a numerical missmatch, but only on Windows. Is it a digression?
• pymc3/tests/test_dist_math.py::test_clipped_beta_rvs[float128] failed for me locally, but I think that's just because float128 is not available on Windows. The test may need a pytest.mark.skipif because some of our CI jobs actually do run on Windows.
• pymc3/tests/test_model_helpers.py::TestHelperFunc::test_pandas_to_array fails because there was a change in tt.as_tensor_variable. The Variable resulting from a ndarray has an .owner that is None. If this was intended by @brandonwillard we can simply take out that assert.
• pymc3/tests/test_examples.py::TestDisasterModel::test_disaster_model currently fails because ArviZ uses a deprecated import path. @OriolAbril will fix this and then we can pin the new ArviZ release here.
• pymc3/tests/test_minibatches.py::TestGenerator::test_gen_cloning_with_shape_change failed because the signature of GeneratorOp.do_constant_folding was incompatible.

[ricardoV94]

@michaelosthege I can check the Weibull test

[ricardoV94]

* [ ]  `pymc3/tests/test_distributions.py::TestMatchesScipy::test_weibull` → relies on pm.math.log1pexp that relies on tt.nnet.softplus. The assert now fails because of numerical reasons. @ricardoV94 maybe you can take a look and decide what to do.

Where did you see that test_weibull relies on log1pexp? I only see it used in the Bernoulli and Logistic distributions. Does the softplus emerge as an optimization after graph compilation?

[twiecki]

Pinging @MarcoGorelli @Spaak for potential help.

[michaelosthege]

* [ ]  `pymc3/tests/test_distributions.py::TestMatchesScipy::test_weibull` → relies on pm.math.log1pexp that relies on tt.nnet.softplus. The assert now fails because of numerical reasons. @ricardoV94 maybe you can take a look and decide what to do.

Where did you see that test_weibull relies on log1pexp? I only see it used in the Bernoulli and Logistic distributions. Does the softplus emerge as an optimization after graph compilation?

You're right. Looks like I messed up something (it was late 🌃 ).
Sorry to bother you about this.

[Spaak]

I'll have a look, starting with test_sampling. I see there already is a Theano-PyMC 1.1.0 lined up; do we still test the fixes here against Theano-PyMC 1.0.15, or should we go to 1.1.0 immediately?

[michaelosthege]

I'll have a look, starting with test_sampling. I see there already is a Theano-PyMC 1.1.0 lined up; do we still test the fixes here against Theano-PyMC 1.0.15, or should we go to 1.1.0 immediately?

On this branch you'll have to test against Theano-PyMC 1.1.0.
I ticked one of the things above and am currently fixing float128 thing. WIll push a commit soon.
You can look into the Weibull thing.

[Spaak]

All failures in test_sampling are also due to the ArviZ deprecated import path (gof). Edit: and, in fact, most test failures appear related to this one.

[Spaak]

pymc3/tests/test_distributions.py::TestMatchesScipy::test_weibull → has a numerical missmatch, maybe because of the upgraded scipy version?

This one succeeds for me locally, with scipy 1.5.0.

[Spaak]

pymc3/tests/test_model_helpers.py::TestHelperFunc::test_pandas_to_array fails because there was a change in tt.as_tensor_variable. The Variable resulting from a ndarray has an .owner that is None. If this was intended by @brandonwillard we can simply take out that assert.

I'm not sure the issue here is with .owner. For me locally, the failing assert is on line 83, assert theano_output == theano_graph_input. The next line involves .owner and actually succeeds.

Some relevant debugging info:

(test_pandas_to_array)>>> theano_output
Elemwise{Cast{int32}}.0

(test_pandas_to_array)>>> theano_graph_input
TensorConstant{[[0 1 2]
 ..
 [6 7 8]]}

(test_pandas_to_array)>>> theano_output.eval()
array([[0, 1, 2],
       [3, 4, 5],
       [6, 7, 8]], dtype=int32)

(test_pandas_to_array)>>> theano_graph_input.eval()
array([[0, 1, 2],
       [3, 4, 5],
       [6, 7, 8]])

(test_pandas_to_array)>>> theano_graph_input.eval().dtype
dtype('int64')

for some reason pm.model.pandas_to_array casts the input to int32 which is causing the assert to fail.

Edit: I think the cast to int32 is deliberate (see lines 1717 and further in model.py), so we should probably update the test to reflect the casting as well. I'll add a commit for this.

[michaelosthege]

I fear the CI tests are useless until we can rely on the new ArviZ version.
I'm on scipy 1.5.2 locally.
But @Spaak and I disagree on test_pandas_to_array: The assert that fails for him was added by me (works for me) so one of us has a broken Theano-PyMC installation? Or less likely, it could be an OS difference? @Spaak on what OS are you?

[Spaak]

@michaelosthege I'm on Ubuntu 20.04.1, running as guest on WSL2 (though that shouldn't matter, I hope). Just in case, the details of my pymc3 and Theano-PyMC repos:

(base) eelke@eelkedesktop:~/repos/pymc3$ git status
On branch pr4405
Your branch is up to date with 'brandonwillard/theano-1.0.15-updates'.

nothing to commit, working tree clean
(base) eelke@eelkedesktop:~/repos/pymc3$ cd ../Theano-PyMC/
(base) eelke@eelkedesktop:~/repos/Theano-PyMC$ git status
HEAD detached at rel-1.1.0
nothing to commit, working tree clean

Edit: and same for current Theano-PyMC master, which has no more commits after rel-1.1.0.

[Spaak]

@twiecki @brandonwillard @MarcoGorelli could one of you perhaps check what you get locally with pymc3/tests/test_model_helpers.py::TestHelperFunc::test_pandas_to_array on Theano-PyMC master (or rel-1.1.0) and this PR branch? @michaelosthege has a failure on line 84 involving .owner (which for me succeeds), but for him the original line 83 (involving a value identity check) succeeded, and failed for me. (Which led me to update the test slightly (last commit 130a96d on this branch).)

I picked you three more or less randomly, so if anyone else drops by and feels like testing this out, feel free :)

[Spaak]

Update: @michaelosthege and I traced this to platform differences in default integer dtype: int32 on Windows (@michaelosthege's platform); int64 on Linux (mine). Which means no need for the three I pinged to check anything; apologies for bothering!

[Spaak]

I pushed the commit, but credit just as much to @michaelosthege; thanks for a great co-coding session :)

[twiecki]

@OriolAbril will fix this and then we can pin the new ArviZ release here.

Is there an arviz issue tracking this?

[michaelosthege]

@OriolAbril will fix this and then we can pin the new ArviZ release here.

Is there an arviz issue tracking this?

arviz-devs/arviz#1495

[twiecki]

So seems like on the PyMC3 side only this one remains:

pymc3/tests/test_distributions.py::TestMatchesScipy::test_weibull → has a numerical missmatch, maybe because of the upgraded scipy version?

Is anyone working on this?

[michaelosthege]

So seems like on the PyMC3 side only this one remains:

pymc3/tests/test_distributions.py::TestMatchesScipy::test_weibull → has a numerical missmatch, maybe because of the upgraded scipy version?

Is anyone working on this?

I don't think so. If you like we can both take a look now.

From #4405 (comment):

This one succeeds for me locally, with scipy 1.5.0.

For me it fails with scipy 1.5.2. So either it's a platform/floatX thing again, or something changed in scipy.

[twiecki]

Great! Sounds like a platform issue to me.

[twiecki]

Hopefully the new arviz issues fixes issue 4.

[twiecki]

  ERROR: Failed building wheel for cftime
ERROR: Could not build wheels for cftime which use PEP 517 and cannot be installed directly

Odd.

[twiecki]

@MarcoGorelli Any idea? Found this pydata/bottleneck#343 (comment)

[Spaak]

Just ran the test suite locally (Ubuntu) with the latest on this branch and latest ArviZ, and we still have some failures (full test log): one is a shape error in test_distributions_random.TestTruncatedNormal (occurs several times, but it's all the same failure); the other one is a simple import rename I think. I added them both to the todo-list here.

The import rename was also caught by CI, but I'm surprised the shape issue was not (I can't seem to find the CI log for test_distributions_random?).

=========================== short test summary info ============================
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormal::test_scalar_distribution_shape[None-None]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormal::test_scalar_distribution_shape[None-()]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormal::test_scalar_distribution_shape[()-None]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormal::test_scalar_distribution_shape[()-()]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormal::test_scalar_sample_shape[None-None]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormal::test_scalar_sample_shape[None-()]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormal::test_scalar_sample_shape[()-None]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormal::test_scalar_sample_shape[()-()]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormalLower::test_scalar_distribution_shape[None-None]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormalLower::test_scalar_distribution_shape[None-()]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormalLower::test_scalar_distribution_shape[()-None]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormalLower::test_scalar_distribution_shape[()-()]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormalLower::test_scalar_sample_shape[None-None]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormalLower::test_scalar_sample_shape[None-()]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormalLower::test_scalar_sample_shape[()-None]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormalLower::test_scalar_sample_shape[()-()]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormalUpper::test_scalar_distribution_shape[None-None]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormalUpper::test_scalar_distribution_shape[None-()]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormalUpper::test_scalar_distribution_shape[()-None]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormalUpper::test_scalar_distribution_shape[()-()]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormalUpper::test_scalar_sample_shape[None-None]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormalUpper::test_scalar_sample_shape[None-()]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormalUpper::test_scalar_sample_shape[()-None]
FAILED pymc3/tests/test_distributions_random.py::TestTruncatedNormalUpper::test_scalar_sample_shape[()-()]
FAILED pymc3/tests/test_variational_inference.py::test_elbo - AttributeError:...

[michaelosthege]

From the diff of this PR, I don't understand why these shape errors appeared.
The AttributeError is easy to fix: use theano.config.change_flags.

The install issue should be fixed now, according to this comment.

[Spaak]

Great to see the CI tests all ✔️ now! I'm still checking out the shape failure I'm getting locally, I propose we hold off merging this PR until I/we figure out what's up with that.

Edit: on a fresh conda env with Python 3.9 and Numpy 1.19.5 and Theano from conda-forge the test succeeds locally for me as well. I'm now checking whether it might be due to Python 3.8 and Numpy 1.19.2 which I'm using in my main conda env.

Edit 2: The test even succeeds on a new conda env with Python/Numpy exactly the same as before. So this is almost certainly some obscure issue with my environment, and I think this can be reviewed and merged now.

[twiecki]

Thanks everyone! Time to cut a new release. @Spaak do you want to do that?