Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixes Pip installed NumPy performance issue. #115

Merged
merged 7 commits into from
Jan 11, 2023
Merged

Conversation

capn-freako
Copy link
Owner

Hey David,

I implemented the proposed solution from that SO post.
Would you test this, please, and let me know what you find?

Thanks!
-db

@capn-freako capn-freako requested a review from jdpatt December 8, 2022 00:44
Copy link
Collaborator

@jdpatt jdpatt left a comment

Choose a reason for hiding this comment

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

I will try it out on both of my machines tonight.

pyproject.toml Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
@jdpatt
Copy link
Collaborator

jdpatt commented Dec 9, 2022

So I ran into some problems that might prevent this being automatic with pip install PyBERT. Trying to install this branch I get this error:

Preparing editable metadata (pyproject.toml) ... done
ERROR: Could not find a version that satisfies the requirement intel-numpy==1.21.4 (from pybert) (from versions: none)
ERROR: No matching distribution found for intel-numpy==1.21.4

Looking at https://pypi.org/project/intel-numpy/ and https://anaconda.org/intel/numpy, you have to tell pip to grab this from the anaconda index not pypi. I've not found how to do this in our pyproject.toml file directly yet. All signs point to it not being supported yet and you have to use the -i flag with pip for now.

So right now, I think we pick a version of numpy that exists in both servers and update our install instructions only. We would have to also make this change in pyibis-ami since otherwise they will have conflicting dependencies.

Should work since intel/simple only has numpy so the rest come from pypi
pip install -i https://pypi.anaconda.org/intel/simple --extra-index-url https://pypi.org/simple PyBERT

or

pip install PyBERT
pip uninstall numpy
pip install -i https://pypi.anaconda.org/intel/simple --extra-index-url https://pypi.org/simple numpy

@capn-freako
Copy link
Owner Author

So I ran into some problems that might prevent this being automatic with pip install PyBERT.

So, I guess the only reason it's working for me is I use conda for virtual environment creation/activation and my pip is supplied by my Anaconda installation and probably doing something "helpful" behind the scenes?

(pybert-dev)
capnf@DESKTOP-G84ND7C MINGW64 ~/Documents/GitHub/PyBERT (jdpatt-feature/gui-changes)
$ which pip
/c/Users/capnf/anaconda3/envs/pybert-dev/Scripts/pip

Copy link
Owner Author

@capn-freako capn-freako left a comment

Choose a reason for hiding this comment

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

@jdpatt , are you going to validate the pip -i ... approach, or should I?

I think it makes more sense for you to try this, since I'm not hitting the error (probably, due to my use of conda).

Thanks,
-db

@jdpatt
Copy link
Collaborator

jdpatt commented Dec 10, 2022

Yeah, I'll keep playing to see if I can get some way that is repeatable across my machines for installing and confirming the unit tests were using the intel-numpy on my machine and not actually installing the normal numpy.

@capn-freako
Copy link
Owner Author

Oh, and I keep forgetting to mention: I do have an old (2012) MacBook Air.
So, I could do some of the Mac experiments, also, to share the work load better on this.
Of course, I won't be testing the new Apple silicon...
... ooh! Maybe I just figured out how to get Wendy to give me the go-ahead to buy a shiny new MacBook for Xmas? ;-)

Happy Holidays, friend!
-db

P.S. - Sorry, one more thing: are you comfortable with a revision to the Faces of PyBERT collage that places us both in the center of the circle and in equally sized frames?

@jdpatt
Copy link
Collaborator

jdpatt commented Dec 15, 2022

So as far as I can tell, you can totally use pip to install numpy-intel but only if you have conda or intel's python distribution since they will do a lot of magic for you behind the scenes to really override pip. I have not had any success and I can only get blas which comes with vanilla numpy when using vanilla python. My speed bump was actually from the first run compared to a byte compiled/cached run. Even piecemeal installing each package, didn't work.

@capn-freako
Copy link
Owner Author

So as far as I can tell, you can totally use pip to install numpy-intel but only if you have conda or intel's python distribution since they will do a lot of magic for you behind the scenes to really override pip. I have not had any success and I can only get blas which comes with vanilla numpy when using vanilla python. My speed bump was actually from the first run compared to a byte compiled/cached run. Even piecemeal installing each package, didn't work.

Sorry, are you saying that now even pip -i ... doesn't work for you?
(I'm having trouble getting the "map" of this issue clear in my head.)

@jdpatt
Copy link
Collaborator

jdpatt commented Dec 17, 2022

Sorry, are you saying that now even pip -i ... doesn't work for you?

Yeah, turns out that was pip falling back to regular numpy and not telling me. Found that np.show_config() will show you BLAS or not.

@capn-freako
Copy link
Owner Author

Okay, so what is your recommendation for bringing this one to closure?

@jdpatt
Copy link
Collaborator

jdpatt commented Dec 19, 2022

I think conda continues to be the one that is pushed in every install guide and if users (like me) still want to use pypi and not conda that we have to live with a performance hit. I don't easily see a way to make these libraries accessible to pypi users of pybert.

I've updated https://github.com/capn-freako/PyBERT/wiki/instant_gratification as such.

@capn-freako
Copy link
Owner Author

Shoot! I just tried the trick proposed by @jdpatt:

pip install pybert
pip uninstall numpy
pip install -i https://pypi.anaconda.org/intel/simple --extra-index-url https://pypi.org/simple numpy==1.22.3

Successfully installed numpy-1.22.3

$ pybert
RuntimeError: module compiled against API version 0x10 but this version of numpy is 0xf
RuntimeError: module compiled against API version 0x10 but this version of numpy is 0xf
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "C:\Users\capnf\.venv\pybert-tst\Scripts\pybert.exe\__main__.py", line 4, in <module>
  File "C:\Users\capnf\.venv\pybert-tst\Lib\site-packages\pybert\cli.py", line 6, in <module>
    from pybert.gui.view import traits_view
  File "C:\Users\capnf\.venv\pybert-tst\Lib\site-packages\pybert\gui\view.py", line 37, in <module>
    from enable.component_editor import ComponentEditor
  File "C:\Users\capnf\.venv\pybert-tst\Lib\site-packages\enable\component_editor.py", line 14, in <module>
    from enable.window import Window
  File "C:\Users\capnf\.venv\pybert-tst\Lib\site-packages\enable\window.py", line 14, in <module>
    Window = toolkit_object("Window")
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\capnf\.venv\pybert-tst\Lib\site-packages\enable\qt4\toolkit.py", line 19, in wrapped
    return func(f'{ETSConfig.kiva_backend}:{name}')
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\capnf\.venv\pybert-tst\Lib\site-packages\pyface\base_toolkit.py", line 127, in __call__
    module = import_module(mname, package)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\capnf\AppData\Local\Programs\Python\Python311\Lib\importlib\__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\capnf\.venv\pybert-tst\Lib\site-packages\enable\qt4\image.py", line 12, in <module>
    from kiva.agg import CompiledPath, GraphicsContextSystem as GraphicsContext
  File "C:\Users\capnf\.venv\pybert-tst\Lib\site-packages\kiva\agg\__init__.py", line 30, in <module>
    from .plat_support import PixelMap
  File "C:\Users\capnf\.venv\pybert-tst\Lib\site-packages\kiva\agg\plat_support.py", line 10, in <module>
    from . import _plat_support
SystemError: initialization of _plat_support raised unreported exception

I think the problem is v1.22.3 is the latest version of the intel-numpy package available, but we've got the PyBERT/PyIBIS-AMI packages pinned to v1.23.5. (v1.24 breaks Chaco.)

I suppose we could try backing off the general package to v1.22.3 and see if anything breaks.
God! This is such a pain. I miss conda just handling all of this. :(

@jdpatt
Copy link
Collaborator

jdpatt commented Dec 30, 2022

If you open up a python terminal and run import numpy as np; np.show_config() does that show openblas or intel-numpy?

I would have it correctly install but it silently fell back to normal numpy.

@capn-freako
Copy link
Owner Author

If you open up a python terminal and run import numpy as np; np.show_config() does that show openblas or intel-numpy?

I would have it correctly install but it silently fell back to normal numpy.

Yeah, I think I'm seeing the same as you:

$ deactivate
$ rm -rf .venv/pybert-tst/
$ python -m virtualenv -p python3.9 .venv/pybert-tst
$ source .venv/pybert-tst/Scripts/activate

(pybert-tst)
capnf@DESKTOP-G84ND7C MINGW64 ~
$ pip install pipbert==4.0.4
$ pip uninstall numpy
$ pip install -i https://pypi.anaconda.org/intel/simple --extra-index-url https://pypi.org/simple "numpy>=1.22.3,<1.23"

(pybert-tst)
capnf@DESKTOP-G84ND7C MINGW64 ~
$ python -c "import numpy as np; print(np.show_config())"
openblas64__info:
    library_dirs = ['D:\\a\\1\\s\\numpy\\build\\openblas64__info']
    libraries = ['openblas64__info']
    language = f77
    define_macros = [('HAVE_CBLAS', None), ('BLAS_SYMBOL_SUFFIX', '64_'), ('HAVE_BLAS_ILP64', None)]
blas_ilp64_opt_info:
    library_dirs = ['D:\\a\\1\\s\\numpy\\build\\openblas64__info']
    libraries = ['openblas64__info']
    language = f77
    define_macros = [('HAVE_CBLAS', None), ('BLAS_SYMBOL_SUFFIX', '64_'), ('HAVE_BLAS_ILP64', None)]
openblas64__lapack_info:
    library_dirs = ['D:\\a\\1\\s\\numpy\\build\\openblas64__lapack_info']
    libraries = ['openblas64__lapack_info']
    language = f77
    define_macros = [('HAVE_CBLAS', None), ('BLAS_SYMBOL_SUFFIX', '64_'), ('HAVE_BLAS_ILP64', None), ('HAVE_LAPACKE', None)]
lapack_ilp64_opt_info:
    library_dirs = ['D:\\a\\1\\s\\numpy\\build\\openblas64__lapack_info']
    libraries = ['openblas64__lapack_info']
    language = f77
    define_macros = [('HAVE_CBLAS', None), ('BLAS_SYMBOL_SUFFIX', '64_'), ('HAVE_BLAS_ILP64', None), ('HAVE_LAPACKE', None)]
Supported SIMD extensions in this NumPy install:
    baseline = SSE,SSE2,SSE3
    found = SSSE3,SSE41,POPCNT,SSE42,AVX,F16C,FMA3,AVX2
    not found = AVX512F,AVX512CD,AVX512_SKX,AVX512_CLX,AVX512_CNL
None

And the observed PyBERT performance is just: 0.6 Msmpls./min. :(

Wow! That was a lot of work for nothing. :(

I'm going to close this.
I think the answer is clear at this point:
Figure out how to successfully install the intel-numpy package.
If that means that you just have to use conda then so be it.

@capn-freako capn-freako closed this Jan 2, 2023
@capn-freako
Copy link
Owner Author

D'oh! I forgot, I do want to merge these changes into master, because if nothing else they'll pin NumPy to a common version usable by both: numpy and intel-numpy.

@jdpatt , I'm about to commit/push.
When I do, would you just give my proposed code changes one final review?
Then, I'll merge into master and we can call this one closed.
Thanks!

@capn-freako capn-freako reopened this Jan 2, 2023
@capn-freako capn-freako requested a review from jdpatt January 2, 2023 20:42
conda.recipe/pybert/meta.yaml Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
conda.recipe/pybert/meta.yaml Show resolved Hide resolved
Copy link
Owner Author

@capn-freako capn-freako left a comment

Choose a reason for hiding this comment

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

Thanks, @jdpatt !
-db

@capn-freako capn-freako merged commit a74a298 into master Jan 11, 2023
@capn-freako capn-freako deleted the numpy-perf branch January 11, 2023 12:44
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants