Skip to content

Conversation

ricardoV94
Copy link
Member

@ricardoV94 ricardoV94 commented Jun 21, 2022

Temporarily running all tests in 3.10, will remove last commit before merging.

Closes #5209

@ricardoV94 ricardoV94 marked this pull request as ready for review June 21, 2022 07:55
@ricardoV94 ricardoV94 marked this pull request as draft June 21, 2022 07:56
Copy link
Member

@michaelosthege michaelosthege left a comment

Choose a reason for hiding this comment

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

@ricardoV94 can you check the check_all_tests_are_covered.py script? It should have failed the pre-commit because several tests are running under two Python versions. Maybe it needs to be updated.

Temporarily running all tests in 3.10, will remove last commit before merging.

I'm positng this as "request changes" as a safeguard against premature merges.

@ricardoV94
Copy link
Member Author

ricardoV94 commented Jun 21, 2022

@ricardoV94 can you check the check_all_tests_are_covered.py script? It should have failed the pre-commit because several tests are running under two Python versions. Maybe it needs to be updated.

I think the script does not check for the new python-version matrix option. But any entry with more than on python-version must by definition result in duplicated tests

@ricardoV94
Copy link
Member Author

ricardoV94 commented Jun 21, 2022

Weird, windows aesara floatX=float64 3.10 failed to install, but aesara floatX=float32 3.10 installed just fine. Flaky conda?

@codecov
Copy link

codecov bot commented Jun 21, 2022

Codecov Report

Merging #5917 (0e75baf) into main (3bf95ce) will increase coverage by 0.05%.
The diff coverage is 96.61%.

❗ Current head 0e75baf differs from pull request most recent head 9491492. Consider uploading reports for the commit 9491492 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5917      +/-   ##
==========================================
+ Coverage   89.50%   89.55%   +0.05%     
==========================================
  Files          73       73              
  Lines       13267    13276       +9     
==========================================
+ Hits        11874    11889      +15     
+ Misses       1393     1387       -6     
Impacted Files Coverage Δ
pymc/distributions/__init__.py 100.00% <ø> (ø)
pymc/smc/sample_smc.py 83.22% <ø> (ø)
pymc/smc/smc.py 98.03% <ø> (+1.57%) ⬆️
pymc/distributions/discrete.py 99.21% <85.71%> (-0.52%) ⬇️
pymc/__init__.py 100.00% <100.00%> (ø)
pymc/aesaraf.py 92.07% <100.00%> (+0.11%) ⬆️
pymc/backends/arviz.py 90.61% <100.00%> (+0.32%) ⬆️
pymc/model.py 88.05% <100.00%> (+0.18%) ⬆️
pymc/sampling_jax.py 96.95% <100.00%> (+0.01%) ⬆️
pymc/step_methods/hmc/nuts.py 97.40% <0.00%> (-0.10%) ⬇️
... and 9 more

@michaelosthege
Copy link
Member

Weird, windows aesara floatX=float64 3.10 failed to install, but aesara floatX=float32 3.10 installed just fine. Flaky conda?

  Encountered problems while solving:
    - package aesara-2.7.3-py37h65a5e79_0 requires python_abi 3.7 *_pypy37_pp73, but none of the providers can be installed

Why is there a 3.7?

@ricardoV94
Copy link
Member Author

Okay there was a typo in the last Windows test, so I expect that to also fail now with 3.10

@ricardoV94
Copy link
Member Author

ricardoV94 commented Jun 21, 2022

So it looks like we have a Windows 10 incompatibility. Can someone try to replicate the issue by creating a test environment with mamba as we do in the CI?

@michaelosthege
Copy link
Member

michaelosthege commented Jun 21, 2022

Nevermind, I ran the wrong env YAML.

@michaelosthege
Copy link
Member

I reproduced the error locally, albeit with inserting - python=3.10 in the .yml manually.

C:\Users\osthege\Repos\pymc-main\conda-envs>mamba env create -f windows-environment-test.yml
conda-forge/win-64                                          Using cache
conda-forge/noarch                                          Using cache
pkgs/r/win-64                                                 No change
pkgs/msys2/win-64                                             No change
pkgs/main/win-64                                              No change
pkgs/r/noarch                                                 No change
pkgs/main/noarch                                              No change
pkgs/msys2/noarch                                             No change


Looking for: ['aeppl=0.0.31', 'aesara=2.7.3', "arviz[version='>=0.12.0']", 'blas', "cachetools[version='>=4.2.1']", 'cloudpickle', "fastprogress[version='>=0.2.0']", "h5py[version='>=2.7']", 'libpython', 'mkl==2020.4', 'mkl-service==2.3.0', 'm2w64-toolchain', "numpy[ve]


Encountered problems while solving:
  - package aesara-2.7.3-py37h65a5e79_0 requires python_abi 3.7 *_pypy37_pp73, but none of the providers can be installed

@michaelosthege
Copy link
Member

It's the mkl-service==2.3.0 dependency.

mamba create --dry-run -c conda-forge -n pmtest "python=3.10" "aeppl=0.0.31" "aesara=2.7.3" "arviz>=0.12" blas "cachetools>=4.2.1" cloudpickle "fastprogress>=0.2.0" "h5py>=2.7" libpython "mkl=2020.4"

mamba create --dry-run -c conda-forge -n pmtest "python=3.10" "aeppl=0.0.31" "aesara=2.7.3" "arviz>=0.12" blas "cachetools>=4.2.1" cloudpickle "fastprogress>=0.2.0" "h5py>=2.7" libpython "mkl=2020.4" "mkl-service==2.3.0"

Encountered problems while solving:
  - package mkl-service-2.3.0-py27h0b88c2a_0 requires python >=2.7,<2.8.0a0, but none of the providers can be installed

Unpinning doesn't help either.

mamba create --dry-run -c conda-forge -n pmtest "python=3.10" "aeppl=0.0.31" "aesara=2.7.3" "arviz>=0.12" blas "cachetools>=4.2.1" cloudpickle "fastprogress>=0.2.0" "h5py>=2.7" libpython "mkl=2020.4" mkl-service

Encountered problems while solving:
  - package mkl-service-1.1.2-py27h0b88c2a_5 requires python >=2.7,<2.8.0a0, but none of the providers can be installed

@ricardoV94
Copy link
Member Author

That looks like something else. Why is it requiring python 2.7?

@ricardoV94
Copy link
Member Author

Seems to have worked!

@michaelosthege
Copy link
Member

That looks like something else. Why is it requiring python 2.7?

This really outdated mkl-service probably didn't pin mkl like the current versions do.
In my commit I removed mkl because it's already a dependency through mkl-service:

package mkl-service-2.4.0-py36h68aa20f_0 requires mkl >=2021.2.0,<2022.0a0, but none of the providers can be installed
                                                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...because of our `mkl=2020.4` pin

Tests were redistributed between Python versions such that all of
3.8, 3.9, 3.10 are covered.

The exact `mkl-service` version was unpinned and `mkl` was removed
because the combination of pinned versions didn't support Python 3.10.
The `mkl` package is already a dependency of `mkl-service` and
because `mkl-service` specifies tight version limits for `mkl`,
we shouldn't pin it ourselves.

Closes pymc-devs#5209
@michaelosthege michaelosthege marked this pull request as ready for review June 21, 2022 18:07
@michaelosthege michaelosthege merged commit 0b4f248 into pymc-devs:main Jun 22, 2022
@michaelosthege michaelosthege added this to the v4.1.0 milestone Jul 2, 2022
@michaelosthege michaelosthege added the major Include in major changes release notes section label Jul 3, 2022
@ricardoV94 ricardoV94 deleted the test_310 branch June 6, 2023 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Github CI/CD maintenance major Include in major changes release notes section release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Run test suite on Python 3.10

3 participants