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

Add posibility to choose python version for module or make it independent from version #16

Closed
yevhen-kalyna opened this issue Mar 24, 2023 · 11 comments

Comments

@yevhen-kalyna
Copy link
Contributor

I had an error after importing the module:

Traceback (most recent call last):
  File "/home/yevhen/Documents/Projects/test/./test.py", line 4, in <module>
    import fastLlama
ImportError: Python version mismatch: module was compiled for Python 3.8, but the interpreter version is incompatible: 3.10.9 (main, Mar 23 2023, 20:52:47) [GCC 9.4.0].

After investigation, I found that in the build folder, the file CMakeCache.txt contains next:

//Path to a program.
PYTHON_EXECUTABLE:FILEPATH=/home/yevhen/anaconda3/bin/python

//Path to a library.
PYTHON_LIBRARY:FILEPATH=/home/yevhen/anaconda3/lib/libpython3.8.so

And its interesting, because my activated venv using python3.10

Can we make this module independent of the version? I know that it is possible to ensure compatibility with any cpython3.x interpreter by only using the Limited API. This way, the package can be compiled against any interpreter version and used with any other, without the need to recompile. More here here

BTW, this issue can be fixed manually by adding arguments to Cmake with needed python versions (DPYTHON_LIBRARY, DPYTHON_EXECUTABLE etc.).

p.s. Thanks for your work. I wanted to create a subprocess to run the model (😂) but found your repository in time.

@yevhen-kalyna
Copy link
Contributor Author

Can be fixed locally with one of:

  1. Reload venv
  2. Passing -DPYTHON_LIBRARY=<Path> -DPYTHON_EXECUTABLE=<Path> to Cmake

This issue is a feature request for avoiding future issues with python versions.

@PotatoSpudowski
Copy link
Owner

You are absolutely right! This should be done with priority!

@amitsingh19975
Copy link
Collaborator

We did consider using the subprocess approach at first, but @PotatoSpudowski disagreed with that approach. Another was to use sockets, but in the end, we settled on creating bindings.

@yevhen-kalyna
Copy link
Contributor Author

@amitsingh19975 Yeah, bindings and models as a module is very useful. I guess in the future we can cover your project under gRPC and some REST-like API to use it as a service. I will share my work with this approach in the next week or two, I guess.

@yevhen-kalyna
Copy link
Contributor Author

BTW, If you will find a way to create a standalone package of fastLLaMA that can be published to pypi, it will be great. I can help with Gitflow and CI/CD for creating releases, in the future, let me know when it's will be needed.

@PotatoSpudowski
Copy link
Owner

That sounds interesting, We can look into it!

@amitsingh19975
Copy link
Collaborator

We added a setup.py that you can use to add command line args to pass your cmake or even set env variables. We can even extend it generates cmake variables to set the python version.

@yevhen-kalyna yevhen-kalyna mentioned this issue Apr 3, 2023
1 task
@PotatoSpudowski
Copy link
Owner

PotatoSpudowski commented Apr 3, 2023

We are going to be fixing this by identifying the python version that is used to run the setup.py file and passing flags accordingly to Cmake in the branch "feature/refactor"

@PotatoSpudowski
Copy link
Owner

This has been added, will merge to main soon.

@yevhen-kalyna could you kindly check the branch and let me know if it is working now!

@yevhen-kalyna
Copy link
Contributor Author

@PotatoSpudowski
As I see now in "feature/refactor" setup.py you are sending -DPYTHON_VERSION={python_version} to cmake, basically Cmake does this automatically and you duplicate this, and a user still can't provide a version which he won't (e.g.).

Also, the original text of this issue says:
Can we make this module independent of the version? I know that it is possible to ensure compatibility with any cpython3.x interpreter by only using the Limited API.

Is the realization of Limited API ready for testing?

p.s. Maybe we should do testing and review using PRs? In this case, I can comment on parts of the code..

@PotatoSpudowski
Copy link
Owner

We now have no pybind11 dependency so this can be closed!

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

3 participants