Update wcEcoli from Python 3.8 to 3.11.3 and up-to-date pip libraries #1367
Conversation
* Use Python 3.11.3 and the latest pip libraries. * For now, this includes numba rc1 and llvmlite rc1, as needed for Python 3.11 compatibility. llvmlite doesn't yet load on Windows. * `np.bool`, `np.int`, and `np.float` are gone. Use `bool` et al. * `open()` mode `"U"` is gone. It was for Python 2. * `bokeh.models.Panel` is now `bokeh.models.TabPanel`. * For PR builds, use pyenv `wcEcoli3-staging`.
* Remove the old numpy stubs and install type stubs via `mypy --install-types`. * mypy no longer allows implicit `Optional[]`. That change caught some bugs. * Don't shadow built-in name `id`. * Fix mixed indentation in `rRNA_operon_expression.py`. (The project style uses tabs but this file uses spaces except one line, so change that line.) * Plotly `append_trace()` is deprecated in favor of `add_trace()`, which is the same except the (unused) return value. **TODO:** "... Bio.Alphabet was therefore removed from Biopython in release 1.78. Instead, the molecule type is included as an annotation on SeqRecords where appropriate." See https://biopython.org/wiki/Alphabet for more information. For now, revert to `biopython==1.77`. "This release of Biopython supports Python 3.6, 3.7 and 3.8" ... so it might not work on 3.11.
Running all `ACTIVE` analysis classes except the `analysisComparison` category mostly produced plausible plots.
* `Ignoring an unknown analysis class: No module named 'models.ecoli.analysis.multigen.new_gene_counts.pypolycistronic_transcription'; 'models.ecoli.analysis.multigen.new_gene_counts' is not a package` -- For want of a `,`.
* (Searching the other `__init__.py` files didn't find another instance of this goof but it's good practice to have a `,` after the last entry in one-entry/line lists so adding another entry makes fewer merge conflicts.)
* `KeyError: 'operons'` in `start_codon_distribution.py`, `metadata['operons']` -- Does a fireworks run set metadata's 'operons' key? `runscripts/manual/runParca.py` does not. Anyway, `sim_data.operons_on` is simple and reliable.
* `UserWarning: color is redundantly defined by the 'color' keyword argument and the fmt string "ob" (-> color='b'). The keyword argument will take precedence.` in `kineticsFluxComparison`.
* `UserWarning: linestyle is redundantly defined by the 'linestyle' keyword argument and the fmt string " " (-> linestyle=' '). The keyword argument will take precedence.` in `growth_condition_comparison_validation.py`.
* Remove `print()` debugging code to remove output clutter. (I recommend the PyCharm debugger. FYI, format strings support a handy debug feature, e.g. `f"Analysis with {exclude_timeout_cells=}, {exclude_early_gens=}"`.)
There are some more warnings like `RuntimeWarning: invalid value encountered in divide`, `RuntimeWarning: divide by zero encountered in divide`, and `No artists with labels found to put in legend.`
Setting `PYTHONWARNINGS=default` should "Show all warnings (even those ignored by default)", in particular more deprecation warnings, although maybe just the first occurrence for each source location. [What's the actual default?] Let's see how useful vs. spammy this is. https://docs.python.org/3/library/warnings.html#the-warnings-filter
* Add pymongo's ocsp dependents to fix Sherlock's access to MongoDB Atlas servers.
(A better error message or a doc could've saved a lot of debugging time.)
* Matplotlib's seaborn- styles are deprecated since 3.6 but live on as `seaborn-v0_8-` styles.
* Speed up `make clean` via `find ... -exec ... {} +`, which is like xargs.
Fireworks hasn't provided release notes in a long time, and it turns out they made at least one incompatible API change: The `LaunchPad()` constructor no longer has `ssl*` arguments. All the SSL and (later) TLS args now go into a `mongoclient_kwargs` arg to pass through to the `pymongo` class `MongoClient`. After digging through their repo to find this change and the Issues on it, it was a reaction to the `pymongo` 4.0 API removing the SSL args in favor of TLS args. Fix here: Call `LaunchPad.from_file()` or `LaunchPad.from_dict()` which extract only the relevant keys from the YAML-provided config. **Recommendation:** Use `uri_mode=True` in your `my_launchpad.yaml` files (and any other way to specify the MongoDB connection). Otherwise, put TLS args into a `mongoclient_kwargs` nested dict. See https://materialsproject.github.io/fireworks/security_tutorial.html#add-tls-ssl-configuration-to-your-launchpad-file
* In a couple cases like `or ''` fixing the type errors fixed runtime problems, but they're probably in unexercised cases. * Fixing `units` required using PEP 647 `TypeGuard`, which is new in Python 3.10. * mypy warnings `By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked]` required adding type signatures to some functions that already contained a little typing for PyCharm's type inspector. Turning on `--check-untyped-defs` would be a larger project.
|
To @rjuenemann and anyone else interested, while waiting for the Numba release it'd be great to test its release candidate (https://pypi.org/project/numba/#history) and the rest of this Python 3.11 PR on Apple Silicon and other platforms. Furthermore, in macOS 13.3, Apple's Accelerate library can now substitute for OpenBLAS and should be considerably faster than OpenBLAS on Apple Silicon. On my Intel Mac, using Accelerate ran Parca + 2 sim gens 6% faster, which is probably just measurement variability. To test with Accelerate: Build one pyenv per
Optional steps to examine/test a pyenv Compare their Parca + sim elapsed CPU run times Optionally compare outputs
Thanks, and go ahead and log results in this PR. |
|
Thank you for the update @1fish2 ! I'll test the 3.11 PR and Accelerate on my M2 and report back. |
|
Hi @1fish2 - I tested this out on my M2 last night. The setup went smoothly and I was able to install the requirements at their specified versions. I did encounter some warnings in the pytest: wcEcoli3.11 wcEcoli3.11-accel Using Accelerate unfortunately did not lead to speed up for me. It does look like numpy linked to accelerate. The Parca outputs seemed to have quite a few differences (7230 lines). I’ve attached the comparison. parcaCompare.txt For runtime comparison on my laptop with the master branch and older Python version: Let me know what you think. I can test on my Windows machine next. |
|
@rjuenemann That's very helpful!
|
|
Hi @1fish2, I’m happy to try to install numpy with I’ve now tried out the Python 3.11 version on my Windows machine. I didn’t run into an issue with Summarize_environment and time_libraries looked okay. I passed all pytests except for failing test_open_blas_threads. Test output attached. Again, I'm not seeing a noticeable difference in runtimes: Compared to old runtimes on this machine: Thanks! |
This will give a quick reading on BLAS performance, in particular whether we've succeeded in getting Apple's Accelerate library to outperform OpenBLAS. Much quicker than running WCM.
|
numba 0.57.0 and llvmlite 0.40.0 were finally released. Now:
Installation notes:
Note: Starting in numpy 1.24.3, integer conversions check for overflow, e.g. |
Update to the newly released versions of ntumba & llvmlite, the first versions compatible with Python 3.11. Get other recent python library updates including numpy 1.24.3.
|
@1fish2 - yes that was Thanks for the update - I will retest with the new library updates. |
|
Hi @1fish2 Retested with the new library updates. I didn’t encounter anything unexpected. Still having warnings with OpenBLAS thread test on M2 and failing on Windows. M2 installing numpy with Accelerate === warnings summary === wholecell/tests/utils/test_openblas_threads.py::Test_openblas_threads::test_openblas ParCa: Elapsed time 263.65 sec (0:04:23.653752); CPU 125.52 sec M2 installing numpy with built-in OpenBLAS === warnings summary === wholecell/tests/utils/test_openblas_threads.py::Test_openblas_threads::test_openblas ParCa: Elapsed time 290.86 sec (0:04:50.857963); CPU 130.23 sec 2-gen simulation: Elapsed time 1291.40 sec (0:21:31.399535); CPU 1291.17 sec Windows installing numpy with built-in OpenBlas OpenBLAS thread test still failing testOpenBlasUpdate.txt ParCa: Elapsed time 447.08 sec (0:07:27.084221); CPU 206.07 sec 2-gen simulation: Elapsed time 2049.56 sec (0:34:09.559818); CPU 2048.94 sec |
|
Thanks, @rjuenemann! I posted to the numpy discussion group asking how to make Accelerate really accelerate. I forked the Chunk library, fixed up the docstrings, and will add it to the repo after writing a unit test; maybe later move it to a PyPI library. Not reproducing the expected dot product means we won't get portable results and thus results in Jenkins builds could differ from those on our development computers. Bummer for debugging. |
|
I'm wondering whether it's worth renaming |
Might not be a bad idea if it's not too big of a hassle |
* Fork the deprecated `chunk` library to fix the deprecation warning. * Add a unit test. * Fold in the .pyi type decls and type Chunk's `file` parameter. * Make `seek()` return the new position, per the `IO` protocol. * Fix relative `seek()` for the case when it's called after reading the pad byte. * Clarify and correct docstrings and comments to match the code and the file format standards. * Switch `tablereader` to use it. Also: Add a tip from @rjuenemann about installing `liblzma_dev` to install in WSL on Windows. TODO: Make `chunk.py` into a PyPI library. Meanwhile, it still uses spaces for indentation since that's the standard in the Python Standard Library and independent libraries.
|
It turns out to use the new BLAS/LAPACK interfaces via macOS Accelerate, numpy has to define the C macro The numpy folks expect a numpy PR to enable that "soon." We could clone the numpy repo and tinker in the meantime or just wait for the PR. |
Interesting. So this could explain why we weren't seeing a substantial speedup using Accelerate? |
|
It seems that compiling with There are lots of variations in installing numpy (e.g. via conda), build details, math operations (should we benchmark matrix multiply?), array size, floating point precision, hardware (M1/M2 -/Pro/Max), and bugs, so I'm unsure about it. |
* Update pips, but stick with `urllib3<2` since 2.0 has an incompatible API which `docker` 6.1.2 doesn't say it's ready for. `requests` 2.30.0 is ready for it, so delay that update. * `sympy==1.12` has [a lot of changes](https://github.com/sympy/sympy/wiki/release-notes-for-1.12). Only a few of them are documented as BREAKING.
* Rename `test_openblas_threads.py` to `test_blas.py`. The test calls numpy, and whether numpy in turn calls OpenBLAS depends on how it was installed. * Don't encourage people to use Apple's Accelerate library yet. Numpy isn't quite ready.
Required-by: ruamel.yaml
1fish2
changed the title
|
@rjuenemann and @ggsun if you escaped the room then please do look at this PR now that it's ready. Is there a more or less convenient time to merge it in? At that time, I'll rebuild the |
|
Thanks @1fish2! I don't have a strong opinion about a convenient time to merge it in - anytime should be fine on my end. |
docs/create-pyenv.md
Outdated
| **Recommendation:** On macOS 13.3+, use | ||
| `pip install numpy==1.24.3 --no-binary numpy` | ||
| to link numpy to the macOS Accelerate library. Otherwise use | ||
| `pip install numpy==1.24.3` | ||
| to use numpy's built-in copy of OpenBLAS. |
There was a problem hiding this comment.
I'm a little confused on the wording in this section vs line 192 in this file - are we recommending Mac users to use Accelerate? Or still use OpenBLAS for now?
There was a problem hiding this comment.
Good catch! I'll remove this until the Numpy team makes it work properly on Accelerate.
Now that the `wcEcoli3` pyenv matches this PR's.
Fixes JuliaLang/julia#1311
Fixes JuliaLang/julia#1365
list[str]andstr | int. (This PR doesn't use the new features.)Python 3.11 is between 10-60% faster than Python 3.10. On average, we measured a 1.25x speedup on the standard benchmark suite. -- That's on top of the 3.9 and 3.10 speedups.
BUT a 1-CPU Sherlock node ran Parca + 2 sim gens only 4% faster. Maybe most of the model's computation is already within numpy, scipy, numba, cython, and aesara.
SHERLOCK ENVIRONMENT MODULES:
openssl/3.0.7to the module/home/groups/mcovert/modules/wcEcoli/python3(for CPython 3.10+) and commented outglpk/4.55-sherlock2andopenblas/0.2.19, leftover from Python 2.TODO
numba==0.57.0+llvmlite==0.40.0when they're released, and re-test. That team has release candidates and they're fixing bugs, esp. on Windows.PYTHONWARNINGS=defaultfromecoli-pull-request.shandrun-fireworks.shif it's too spammy.wcEcolipyenv before merging into master. (This PR useswcEcoli-staging.)