hansalemaos / cythoncartesian

Cartesian Product - 6x faster than itertools.product - 10x less memory
https://pypi.org/project/cythoncartesian
MIT License
1 stars 0 forks source link

cant get it working #1

Open gulizi opened 7 months ago

gulizi commented 7 months ago

unable to import, importing the package will spawn another python interpreter, kind of weird

from cythoncartesian import cartesian_product Python 3.11.8 | packaged by conda-forge | (main, Feb 16 2024, 20:53:32) [GCC 12.3.0] on linux Type "help", "copyright", "credits" or "license" for more information.

hansalemaos commented 7 months ago

It is trying to compile the code. That happens only once at the beginning, but I haven't tested it with Linux. Could you please try this and tell me if it works: https://github.com/hansalemaos/numpycythonpermutations I used a new way of compiling the code, and this module does what you want faster than this here and has some extra stuff. You have a C compiler, right? And Cython is working correctly, right?

gulizi commented 7 months ago

thanks for the fast reply, still similar errors. I am on ubuntu, have cpython and gcc.

On Thursday, March 7, 2024 at 12:31:47 PM EST, Hans Alemão ***@***.***> wrote:  

It is trying to compile the code. That happens only once at the beginning, but I haven't tested it with Linux. Could you please try this and tell me if it works: https://github.com/hansalemaos/numpycythonpermutations I used a new way of compiling the code, and this module does what you want faster than this here and has some extra stuff. You have a C compiler, right? And Cython is working correctly, right?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

hansalemaos commented 7 months ago

Have you ever tested Cython (not CPython)? Install it and run a test compilation: https://cython.org/

By the way numpycythonpermutations is compiled with c++ 20 / openmp

gulizi commented 7 months ago

I've used cython in sage environment, but I will test cpython in python environment. I have to do it later tonight though, will get back to you.Thanks again for the quick helps!  On Thursday, March 7, 2024 at 01:01:32 PM EST, Hans Alemão @.***> wrote:

Have you ever tested Cython (not CPython)? Install it and run a test compilation: https://cython.org/

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

hansalemaos commented 7 months ago

No problem! :) Waiting for your answer tonight

gulizi commented 7 months ago

I tested my Cython, seems fine right? (sage) @.:~$ more test_cython.pyx import time cdef unsigned long long int maxvalcdef unsigned long long int totalcdef int kcdef float t1, t2, t maxval=1000000000 t1=time.time() for k in range(maxval):    total = total + kprint "Total =", total t2=time.time()t = t2-t1print("%.100f" % t)(sage) @.:~$ vi test_cython.pyx (sage) @.:~$ python setup.py build_ext --inplaceCompiling test_cython.pyx because it changed.[1/1] Cythonizing test_cython.pyx/home/guobin/mambaforge/envs/sage/lib/python3.11/site-packages/Cython/Compiler/Main.py:381: FutureWarning: Cython directive 'language_level' not set, using '3str' for now (Py3). This has changed from earlier releases! File: /home/guobin/test_cython.pyx  tree = Parsing.p_module(s, pxd, full_module_name)(sage) @.:~$ pythonPython 3.11.8 | packaged by conda-forge | (main, Feb 16 2024, 20:53:32) [GCC 12.3.0] on linuxType "help", "copyright", "credits" or "license" for more information.>>> import test_cythonTotal = 4999999995000000000.0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000>>> 

On Thursday, March 7, 2024 at 01:01:32 PM EST, Hans Alemão ***@***.***> wrote:  

Have you ever tested Cython (not CPython)? Install it and run a test compilation: https://cython.org/

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

hansalemaos commented 7 months ago

Yes, seems right. Can you check, if your compiler accepts these directives:

"/O2", "/Oy", "/std:c++20", "/openmp"

gulizi commented 7 months ago

First seems have to be -option not /option, second having problem with Oy, third std should be std= instead of std:is this the reason import failing?  (sage) @.:~$ gcc hello.c -fopenmp -o hello(sage) @.:~$ gcc hello.c -fopenmp -Oy -o hellocc1: error: argument to ‘-O’ should be a non-negative integer, ‘g’, ‘s’ or ‘fast’(sage) @.:~$ gcc hello.c -fopenmp -O2 -o hello(sage) @.:~$ gcc hello.c -fopenmp -O2 /Oy -o hello/usr/bin/ld: cannot find /Oy: No such file or directorycollect2: error: ld returned 1 exit status(sage) @.:~$ gcc hello.c -fopenmp -O2 /O2 -o hello/usr/bin/ld: cannot find /O2: No such file or directorycollect2: error: ld returned 1 exit status(sage) @.:~$ gcc hello.c -fopenmp -O2 -std:c++20 -o hellogcc: error: unrecognized command-line option ‘-std:c++20’; did you mean ‘-std=c++20’?(sage) @.***:~$ gcc hello.c -fopenmp -O2 -std=c++20 -o hellocc1: warning: command-line option ‘-std=c++20’ is valid for C++/ObjC++ but not for C

On Thursday, March 7, 2024 at 09:44:15 PM EST, Hans Alemão ***@***.***> wrote:  

Yes, seems right. Can you check, if your compiler accepts these directives:

"/O2", "/Oy", "/std:c++20", "/openmp"

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

hansalemaos commented 7 months ago

I guess that is the problem, I think it is MSVC especific. You can change the source code: https://github.com/hansalemaos/numpycythonpermutations/blob/main/__init__.py

This line: "extra_compile_args": ["/O2", "/Oy", "/std:c++20", "/openmp"], To whatever your compiler accepts. It should work then.

gulizi commented 7 months ago

does the compile happen at the import time or pip install time? I reinstalled the package with the compiler option modified sources and saw no errors, but didn't help.  On Thursday, March 7, 2024 at 09:44:15 PM EST, Hans Alemão @.***> wrote:

Yes, seems right. Can you check, if your compiler accepts these directives:

"/O2", "/Oy", "/std:c++20", "/openmp"

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

hansalemaos commented 7 months ago

It tries to import the cython stuff, if it doesn't work, it compiles the pyx file, and tries to import again. Did it create new files in the folder of the package?

gulizi commented 7 months ago

so the pip install just copy the source files, and compile happens on the fly at import time if needed? By the folder of the package you mean in the site-packages/numpycythonpermutations directory? I see no new files except init.cpython-311.pyc in pycache directory

hansalemaos commented 7 months ago

It should look similar to that: https://www.youtube.com/watch?v=iNkKGPclp5Y 3 possible reasons: 1) I am using something in the config that is not supported by GCC 2) Some rights issues to save the files 3) \r\n instead of \n maybe?

gulizi commented 7 months ago

your video is helpful. I traced the code to compile_cython_code , which spawn python interpreter instead of compiling. Where is the function compile_cython_code, I coudn't find any reference or source code to it. If I can find it then I can trace futher down.

hansalemaos commented 7 months ago

This one here: https://github.com/hansalemaos/cycompi It should be installed automatically

gulizi commented 7 months ago

I found why compile_cython_code is spawning another interpreter, it is because of the shell=True parameter, but I don't know what is the original intension of shell = True

subprocess.run([ 'python', 'setup.py', 'build_ext', '--inplace']) CompletedProcess(args=['python', 'setup.py', 'build_ext', '--inplace'], returncode=0) subprocess.run([ 'python', 'setup.py', 'build_ext', '--inplace'], shell=True) Python 3.11.8 | packaged by conda-forge | (main, Feb 16 2024, 20:53:32) [GCC 12.3.0] on linux Type "help", "copyright", "credits" or "license" for more information.

hansalemaos commented 7 months ago

It spans a shell, try to set it to False

gulizi commented 7 months ago

it starts compiling fine, but the fail at ld: cannot find build/temp.linux-x86_64-cpython-311/home/***/mambaforge/envs/sage/lib/python3.11/site-packages/numpycythonpermutations/uniqueproductcython.o seems the path is messed up because it has /home/myusername in the middel

hansalemaos commented 7 months ago

I think, I know what causes the problem! In Windows, the ending is pyd and in Linux so/o https://cython.readthedocs.io/en/latest/src/userguide/source_files_and_compilation.html Try renaming the file in the source code to so or, probably better, o. Then it should work out.

gulizi commented 7 months ago

ok I finally figured it out, even though I got the compile working, the option I changed to -openmp actually compile the file with output penmp because it is interpreted as "-o penmp" , after I get rid of -openmp it works. I think the only change need to make your package working is the shell=False because /openmp wouldn't cause this. Thanks a lot for your help!

hansalemaos commented 7 months ago

I thank you very much for testing the module on Linux. As far as I remember, I needed the shell=True on Windows. Usually, I avoid shell=True. In the next couple of days, I will update the module and add a platform check. Have you gotten a good speedup on Linux? I would like to know if it runs better than on Windows.

gulizi commented 7 months ago

I switched my algorithm so big product is no longer used. I run your sample code, this is what I get: sage: %timeit resus = np.array(list(itertools.product(*args)))4.69 s ± 8.45 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)sage: %timeit resus = generate_product(args, remove_duplicates=False, str_format_function=repr, multicpu=True, return_index_only=False, max_reps_rows=-1, r=-1, dummyval="DUMMYVAL")671 ms ± 6.84 ms per loop (mean ± std. dev. of 7 runs, 1 loop each) sage: %timeit resus = generate_product(args, remove_duplicates=False, str_format_function=repr, multicpu=True, return_index_only=False, max_reps_rows=-1, r=-1, dummyval="DUMMYVAL")668 ms ± 5.14 ms per loop (mean ± std. dev. of 7 runs, 1 loop each) sage: %timeit resus = generate_product(args, remove_duplicates=False, str_format_function=repr, multicpu=True, return_index_only=False, max_reps_rows=-1, r=-1, dummyval="DUMMYVAL")668 ms ± 5.46 ms per loop (mean ± std. dev. of 7 runs, 1 loop each) significant improvement but not as good as yours, maybe because some compiler option not working correctly.

On Saturday, March 9, 2024 at 09:07:54 PM EST, Hans Alemão ***@***.***> wrote:  

I thank you very much for testing the module on Linux. As far as I remember, I needed the shell=True on Windows. Usually, I avoid shell=True. In the next couple of days, I will update the module and add a platform check. Have you gotten a good speedup on Linux? I would like to know if it runs better than on Windows.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>