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

🐛 Bug: Bentoml and Python3.12 compatibility #1379

Closed
Abellegese opened this issue Nov 13, 2024 · 22 comments · Fixed by #1498
Closed

🐛 Bug: Bentoml and Python3.12 compatibility #1379

Abellegese opened this issue Nov 13, 2024 · 22 comments · Fixed by #1498
Assignees
Labels
bug Something isn't working

Comments

@Abellegese
Copy link
Contributor

Abellegese commented Nov 13, 2024

Describe the bug.

Recently got this error when fetching eos3b5e from a github on python 3.12 conda env and got this error:

  File "/home/abellegese/miniconda3/lib/python3.12/site-packages/pkg_resources/__init__.py", line 2191, in <module>
    register_finder(pkgutil.ImpImporter, find_on_path)
                    ^^^^^^^^^^^^^^^^^^^
AttributeError: module 'pkgutil' has no attribute 'ImpImporter'. Did you mean: 'zipimporter'?
🚨🚨🚨 Something went wrong with Ersilia 🚨🚨🚨

Error message:

Expecting value: line 1 column 1 (char 0)
If this error message is not helpful, open an issue at:
 - https://github.com/ersilia-os/ersilia
Or feel free to reach out to us at:
 - hello[at]ersilia.io

If you haven't, try to run your command in verbose mode (-v in the CLI)
 - You will find the console log file in: /home/abellegese/eos/current.log

Describe the steps to reproduce the behavior

  1. Create a fresh ersilia environment with Python 3.12
  2. Try to fetch the model from source, something like ersilia -v fetch eos3b5e --from_github.

Operating environment

Ubuntu

@Abellegese Abellegese added the bug Something isn't working label Nov 13, 2024
@DhanshreeA DhanshreeA self-assigned this Nov 14, 2024
@DhanshreeA
Copy link
Contributor

@Abellegese I remember the stack trace having more to it - ie specifically, I am asking for the part of the trace where this bug originates from ersilia's copy of bentoml. If possible could you attach that to the issue as well?

@Abellegese
Copy link
Contributor Author

Okay @DhanshreeA will update it.

@GemmaTuron GemmaTuron changed the title 🐛 Bug: Bentoml and Python3.12 comatibility 🐛 Bug: Bentoml and Python3.12 compatibility Nov 28, 2024
@GemmaTuron GemmaTuron moved this from On Hold to In Progress in Ersilia Model Hub Nov 28, 2024
@GemmaTuron
Copy link
Member

Hi @DhanshreeA and @Abellegese

Can you update this issue? And set a deadline for completion

@DhanshreeA DhanshreeA moved this from In Progress to On Hold in Ersilia Model Hub Nov 29, 2024
@DhanshreeA DhanshreeA removed their assignment Nov 29, 2024
@DhanshreeA DhanshreeA self-assigned this Dec 16, 2024
@DhanshreeA
Copy link
Contributor

DhanshreeA commented Dec 17, 2024

I am unable to reproduce the exact error that @Abellegese has reported here, however, I do notice an issue with Python 3.12 and fetching from github, however I do experience a similar error (ie in that it is coming from pkg_resources module):

Traceback (most recent call last):
  File "/home/dee/.pyenv/versions/ersilia-312/bin/bentoml", line 5, in <module>
    from bentoml.cli import cli
  File "/home/dee/.pyenv/versions/3.12.5/envs/ersilia-312/lib/python3.12/site-packages/bentoml/__init__.py", line 27, in <module>
    from bentoml.saved_bundle import load_from_dir, save_to_dir  # noqa: E402
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/dee/.pyenv/versions/3.12.5/envs/ersilia-312/lib/python3.12/site-packages/bentoml/saved_bundle/__init__.py", line 15, in <module>
    from bentoml.saved_bundle.bundler import save_to_dir
  File "/home/dee/.pyenv/versions/3.12.5/envs/ersilia-312/lib/python3.12/site-packages/bentoml/saved_bundle/bundler.py", line 31, in <module>
    from bentoml.saved_bundle.local_py_modules import (
  File "/home/dee/.pyenv/versions/3.12.5/envs/ersilia-312/lib/python3.12/site-packages/bentoml/saved_bundle/local_py_modules.py", line 27, in <module>
    from bentoml.saved_bundle.pip_pkg import get_all_pip_installed_modules
  File "/home/dee/.pyenv/versions/3.12.5/envs/ersilia-312/lib/python3.12/site-packages/bentoml/saved_bundle/pip_pkg.py", line 23, in <module>
    from pkg_resources import Requirement
ModuleNotFoundError: No module named 'pkg_resources'

Edit: This was in a fresh ersilia environment created using Python 3.12, and then running: ersilia fetch eos3b5e --from_github.

@DhanshreeA
Copy link
Contributor

Okay, I believe we have run into this before. However, this only fixed ersilia code using the pkg_resources module - which doesn't address this issue coming up from bentoml.

@DhanshreeA DhanshreeA moved this from On Hold to In Progress in Ersilia Model Hub Dec 17, 2024
@DhanshreeA DhanshreeA moved this from In Progress to On Hold in Ersilia Model Hub Dec 18, 2024
@DhanshreeA DhanshreeA removed their assignment Dec 18, 2024
@DhanshreeA
Copy link
Contributor

Downgrading the priority here for two reasons:

  1. In Python 3.12, models will work if fetched from Docker
  2. We will try to do a massive refactoring of all models to update their installation files (from Dockerfile to install.yml) and hopefully get rid of bentoml completely - In fact this could be our 2.0.0 release (@miquelduranfrigola)

@DhanshreeA
Copy link
Contributor

DhanshreeA commented Dec 19, 2024

Another update:

We will continue to use our BentoML fork for packaging models, however, we will utilise Ersilia Pack for as many old models, and all new models. This is necessary because Ersilia Pack utilises FastAPI which only supports Python 3.8 and above. We will refactor most models to use an upgraded Python version especially if they use Python 3.7 or below. However, for those models that cannot be upgraded, and are still important for us to keep in the hub, we will continue to utilise BentoML.

This scenario merits adding to the code, a conditional check here that checks if the user is running ersilia in a Python 3.12 (or above) environment - in this case, if the user is fetching a model from source, ie GitHub, local path, or S3, we will force packing this model using Ersilia Pack.

There's a caveat to the above strategy, that is, if the model dependency has a version less than 3.8, then we just simply raise an error to the user that Ersilia should be run in an environment less than 3.12, and exit.

@DhanshreeA
Copy link
Contributor

@OlawumiSalaam is this something that interests you?

@OlawumiSalaam
Copy link
Contributor

@DhanshreeA
Hi. I am unable to reproduce the error as I am able to fetch the model eos3b5e from source using this CLI ersilia -v fetch eos3b5e --from_github in python --version
Python 3.12.8.
I logged the output as
fetch_log.txt
Please did I miss any step?

@OlawumiSalaam
Copy link
Contributor

Here are the steps I followed to reproduce the bug

  1. I switched to the ersilia directory:
    cd ersilia
    To ensure I am working with the latest version of the ersilia repo
    git pull origin master
  2. created a new Conda environment named ersilia_python3.12 with Python 3.12
    conda create -n ersilia_python3.12 python=3.12
  3. activated the newly created environment:
    conda activate ersilia_python3.12
  4. I installed the Ersilia CLI by running
    pip install .
  5. fetch the model eos3b5e from GitHub using the Ersilia CLI and logged the output to fetch_log.txt
    ersilia -v fetch eos3b5e --from_github > fetch_log.txt 2>&1
  6. The fetch operation was successful, and no errors were encountered during this process as shown in the
    fetch_log.txt
    Did I miss any step?

@Abellegese
Copy link
Contributor Author

Abellegese commented Jan 6, 2025

Hi @OlawumiSalaam both of the logs shows that the fetching was skipped because the model eos3b5e was already in your system. Just delete the model and then refetch it again. When it decides to use bentoml to pack the model that error in principle has to happen.

@OlawumiSalaam
Copy link
Contributor

@Abellegese . Thank you for pointing that out. I have deleted and fetched again. Here is the output
fetch_log.txt

@Abellegese
Copy link
Contributor Author

Okay thanks @OlawumiSalaam you are right. I think pkg_resource is now totally removed from newest py3.12 and versions. Here is the link:

"Do not pre-install setuptools in virtual environments created with venv. This means that distutils, setuptools, pkg_resources, and easy_install will no longer available by default; to access these run pip install setuptools in the activated virtual environment."
https://docs.python.org/3/whatsnew/3.12.html#:~:text=Do%20not%20pre%2Dinstall%20setuptools%20in%20virtual%20environments%20created%20with%20venv.%20This%20means%20that%20distutils%2C%20setuptools%2C%20pkg_resources%2C%20and%20easy_install%20will%20no%20longer%20available%20by%20default%3B%20to%20access%20these%20run%20pip%20install%20setuptools%20in%20the%20activated%20virtual%20environment.

@DhanshreeA DhanshreeA moved this from On Hold to In Progress in Ersilia Model Hub Jan 7, 2025
@OlawumiSalaam
Copy link
Contributor

Bug: The issue stems from pkg_resources incompatibility with Python 3.12 due to the removal of pkgutil.ImpImporter.
Python 3.12 removed pkgutil.ImpImporter and also stopped pre-installing setuptools in venv environments.
Observations:

  • I successfully fetched the model because my environment had setuptools properly installed because I created the environment with Conda
  • Python 3.12 requires explicit installation of setuptools in virtual environments.

Suggestions:

  • Add checks to detect Python 3.12 environments and prompt users to install setuptools.
  • Update documentation to inform users about installing setuptools in Python 3.12 environments.

@DhanshreeA
Copy link
Contributor

@OlawumiSalaam both good points. Let's do this, add a check for the python version in the code, and try to import setuptools, if there's an import error, install setuptools (while also logging this on the CLI). And you can also document this in the README in your PR.
We don't need to do any user prompting for installation, we can just install it in the code, but we do need to log this. You can take inspiration for how we do this for rdkit, for example, from here.

@OlawumiSalaam
Copy link
Contributor

@DhanshreeA I am a bit confused and need guide.
Do you mean that I should create a class that handles the setuptools installation in a python file and save it the requirements dir or
should I add the checks and installation of setuptools within the code.
Also, should the steps be for when fetching models from BentoML

@DhanshreeA
Copy link
Contributor

Hi @OlawumiSalaam

Having a class would be an overkill, I was referring to the rdkit implementation as a workflow to look at while doing this - in that it first tries to import the module to check if it is present, and if not, uses the ersilia utility run_command to pip install it.

We don't need to have a full blown class, we can simply do it within the code that you have referenced. I think it's safe to do it within the fetch at the beginning (regardless of whether it's bentoml, or fastapi), just to be safe.

@DhanshreeA
Copy link
Contributor

Okay, so good work @OlawumiSalaam I have tested the changes in this PR for eos3b5e in a Python 3.12 environment (regular venv, not conda). I propose you test more models mainly the ones listed here and share updates here so we can close this issue for good. Thanks, and good work!

@OlawumiSalaam
Copy link
Contributor

OlawumiSalaam commented Jan 13, 2025

@DhanshreeA I have tried fetching models but not successful. I don,t know what I am doing wrong. Here is the output
output.txt.
eos4e40_output.txt

please can you take a look

@DhanshreeA
Copy link
Contributor

DhanshreeA commented Jan 13, 2025

@OlawumiSalaam Looking at these logs, I believe the cause for this error is that somehow the model packages are not getting installed in the isolated conda environment that gets created during the fetch procedure and are instead somehow being installed in ersilia's virtual environment.

For example, for eos3b5e the rdkit version very well exists for Python 3.10, which is the base Python for the model, however here pip complains:

ERROR: Could not find a version that satisfies the requirement rdkit==2022.9.5 (from versions: 2023.9.1, 2023.9.2, 2023.9.3, 2023.9.4, 2023.9.5, 2023.9.6, 2024.3.1, 2024.3.2, 2024.3.3, 2024.3.4, 2024.3.5, 2024.3.6, 2024.9.1, 2024.9.2, 2024.9.3, 2024.9.4)
ERROR: No matching distribution found for rdkit==2022.9.5

It is trying to find an rdkit version for Python 3.12 (which is the python in Ersilia's environment.)

I am not sure why this is happening in your system, it's probably a path corruption somehow. In that, when the model install commands are running on your system, the pip package ends up corresponding to pip from your ersilia environment and not the model environment for some reason.

I believe a similar thing is happening for eos4e40 as well. You could try examining your system path to begin with. In any case, I can take over testing these models.

@DhanshreeA
Copy link
Contributor

DhanshreeA commented Jan 13, 2025

Tracking models here:

  • eos3b5e
  • eos4e40
  • eos7d58 - Doesn't fetch but no setuptools error, instead has a Numpy version error.
  • eos9gg2 - Doesn't fetch, but no setuptools issue. The issue is similar to this.
  • eos3cf4
  • eos7w6n - Issue with standard example for some reason during fetching
  • eos4wt0 - Similar issue as eos9gg2
  • eos2gw4
  • eos7jio - Some issue with refactoring leading to this error: AttributeError: 'NoneType' object has no attribute 'checkpoints_dir'
  • eos5axz - Similar issue as eos7jio, due to the same kind of refactoring required by both these models.
  • eos4u6p - Standard example issue.

@DhanshreeA
Copy link
Contributor

It looks like the setuptools issue is resolved through the linked PR, and wherever models failed to fetch is because of other reasons. We can safely close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants