Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

cant get it working #1

Open
gulizi opened this issue Mar 7, 2024 · 22 comments
Open

cant get it working #1

gulizi opened this issue Mar 7, 2024 · 22 comments

Comments

@gulizi
Copy link

gulizi commented Mar 7, 2024

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
Copy link
Owner

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
Copy link
Author

gulizi commented Mar 7, 2024 via email

@hansalemaos
Copy link
Owner

hansalemaos commented Mar 7, 2024

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
Copy link
Author

gulizi commented Mar 7, 2024 via email

@hansalemaos
Copy link
Owner

No problem! :) Waiting for your answer tonight

@gulizi
Copy link
Author

gulizi commented Mar 8, 2024 via email

@hansalemaos
Copy link
Owner

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

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

@gulizi
Copy link
Author

gulizi commented Mar 8, 2024 via email

@hansalemaos
Copy link
Owner

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
Copy link
Author

gulizi commented Mar 9, 2024 via email

@hansalemaos
Copy link
Owner

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
Copy link
Author

gulizi commented Mar 9, 2024

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
Copy link
Owner

hansalemaos commented Mar 9, 2024

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
Copy link
Author

gulizi commented Mar 9, 2024

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
Copy link
Owner

hansalemaos commented Mar 9, 2024

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

@gulizi
Copy link
Author

gulizi commented Mar 9, 2024

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
Copy link
Owner

It spans a shell, try to set it to False

@gulizi
Copy link
Author

gulizi commented Mar 9, 2024

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
Copy link
Owner

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
Copy link
Author

gulizi commented Mar 10, 2024

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
Copy link
Owner

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
Copy link
Author

gulizi commented Mar 10, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants