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

text_prompts.ipynb fails on Initialize LangSAM class #85

Closed
p-vdp opened this issue May 24, 2023 · 9 comments · Fixed by #87 or #93
Closed

text_prompts.ipynb fails on Initialize LangSAM class #85

p-vdp opened this issue May 24, 2023 · 9 comments · Fixed by #87 or #93
Labels
bug Something isn't working

Comments

@p-vdp
Copy link
Collaborator

p-vdp commented May 24, 2023

Environment Information

  • samgeo version: 0.8.0
  • Python version: 3.9.16
  • Operating System: Windows 11
  • ArcGIS Pro conda env set up using this repo's instructions

Description

Tried to run through the notebook. At step "Initialize LangSAM class" it throws a NameError on hf_hub_download.

image

What I Did

Restarted the kernel and Arc and tried again.
Installed huggingface_hub in the env via mamba, restarted kernel and tried again.

Looks like there's an issue with importing huggingface_hub in text_sam.py module. Should groundingdino and huggingface_hub be added to requirements.txt etc?

@p-vdp p-vdp added the bug Something isn't working label May 24, 2023
@p-vdp p-vdp changed the title text_prompts.ipynb text_prompts.ipynb fails on Initialize LangSAM class May 24, 2023
@giswqs
Copy link
Member

giswqs commented May 24, 2023

groundingdino is not yet available on conda-forge. It can't be added to requirements.txt as the conda-forge package will fail to build if it is included.

huggingface_hub can potentially be added. I will look into it.

@lukesteinbicker
Copy link
Contributor

I looked in the package files and found that there is only one class, name "SamGeo". If you write "from samgeo import SamGeo", your issues should be resolved. I assume that there may have been a seperate class named "LangSAM" that was deprecated.

@giswqs giswqs linked a pull request May 25, 2023 that will close this issue
@giswqs
Copy link
Member

giswqs commented May 25, 2023

huggingface_hub has been added to requirements.txt. #87

GroundingDINO and supervision are being added to conda-forge (conda-forge/staged-recipes#22907). It can be added to requirements.txt once they become available on conda-forge.

@p-vdp
Copy link
Collaborator Author

p-vdp commented May 27, 2023

@lukesteinbicker

I looked in the package files and found that there is only one class, name "SamGeo". If you write "from samgeo import SamGeo", your issues should be resolved. I assume that there may have been a seperate class named "LangSAM" that was deprecated.

I like how you're thinking but LangSAM is a class in samgeo/text_sam.py, thus the import statement in text_prompts.ipynb:

from samgeo.text_sam import LangSAM

I updated to 0.8.1 and now get an OSError thrown by GroundingDINO:

File C:\Users\username\AppData\Local\ESRI\conda\envs\arcgispro-py3-samgeo\lib\site-packages\groundingdino\util\slconfig.py, in _file2dict:

Line 104: raise IOError("Only py/yml/yaml/json type are supported now!")

OSError: Only py/yml/yaml/json type are supported now!

Looks to be something with what LangSAM is using as cache_config_file in SLConfig.fromfile(cache_config_file)

image

@p-vdp p-vdp reopened this May 27, 2023
@p-vdp
Copy link
Collaborator Author

p-vdp commented May 28, 2023

I think I found the source of the problem with text_sam.load_model_hf()

When I run the huggingface_hub.hf_hub_download function in the ArcGIS Pro notebook, the cached file path is something like this:

C:\\Users\\username/.cache\\huggingface\\hub\\c593cfe8389419176fe6d2a7b3fb5f2a87fb4a45a7c780ef37813742c4910f49.3b220b33a5e67bbef5a685c9dd805f05aa7f36bc592cf143628033f6f6c7e61b

Not sure why this is in a hashed/hex(?) form but obviously that's why SLConfig doesn't see a .py file.

If we use the force_filename parameter with hf_hub_download() we can ensure the output filename is the same as the input. Applicable here and here.

@NipunaChh
Copy link

Screenshot from 2023-12-04 15-26-54
So how do I import LangSam, I seem to be running into a similar error, "NameError: name 'SLConfig' is not defined"

@vxi214
Copy link

vxi214 commented Dec 14, 2023

图片1,Boss, I also encountered this problem, how to solve it?

@icekang
Copy link

icekang commented Feb 16, 2024

Screenshot from 2023-12-04 15-26-54 So how do I import LangSam, I seem to be running into a similar error, "NameError: name 'SLConfig' is not defined"

I solved SLConfig is not defined by directly cloning GroundingDINO repo and install it directly.

# uninstall groundingdino-py
pip uninstall groundingdino-py

# install GroundingDINO from github
git clone https://github.com/IDEA-Research/GroundingDINO.git
cd GroundingDINO/
pip install -e .

@krishnaglodha
Copy link

How is this resolved ?

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
None yet
7 participants