search

Kohulan / DECIMER-Image_Transformer

DECIMER: Deep Learning for Chemical Image Recognition using Efficient-Net V2 + Transformer
MIT License
197 stars 51 forks source link

Allow to define the location of the downloaded model #70

Closed j3mdamas closed 9 months ago

j3mdamas commented 1 year ago

Issue Type

Feature Request

Source

GitHub (source)

DECIMER Image Transformer Version

2.3.0

OS Platform and Distribution

No response

Python version

No response

Current Behaviour?

I would like to change the behavior of the model download (present in https://github.com/Kohulan/DECIMER-Image_Transformer/blob/master/DECIMER/decimer.py#L28-L52) to allow the definition of a global location that can be pre-downloaded and used by multiple users on the same system.

I can make a pull request with that feature. My intervention on the code would be minimal: I would not use any configuration library, I would decide a environment variable name (like DECIMER_MODEL_PATH), check its existence at runtime, and depending on it being defined, use it as the base path or default to the current system. I would also document that variable and usage on the README.

Which images caused the issue? (This is mandatory for images related issues)

No response

Standalone code to reproduce the issue

There is no issue, this is a feature request

Relevant log output

No response

Code of Conduct

Kohulan commented 1 year ago

Hi @j3mdamas ,

Thank you for the suggestion. It would be a great addition. If possible, you can create a pull request. I will merge it if all tests pass and it doesn't affect decimer.ai docker image.

Kind regards, Kohulan

j3mdamas commented 1 year ago

Hi, I looked at it yesterday, and what I wanted to do can already be done with the pystow module, using PYSTOW_HOME, so I stopped immediately. I still pythonified a bit that part of the code and created a small documentation update for using another location for the model. Do you want a PR for that? I was also thinking about adding that part of the code to download the model to DECIMER.utils as a function, and calling it in decimer.py. This would allow an explicit download of the model as:

from DECIMER.utils import download_model
download_model()

I know the above can be achieved by doing an import predict_SMILES, but this way it's more explicit and encapsulated.

j3mdamas commented 9 months ago

@Kohulan I will assume you are not interested in the PR and I will delete my local branch,

Kohulan commented 9 months ago

@j3mdamas I am happy to get the PR but haven't received any, since there has been no activity on this issue for the last 3 months since I closed it. If you still have the branch please do make the pull request, and I will check it. Thanks a lot.

j3mdamas commented 9 months ago

@Kohulan I guess we were waiting on each other then :)

Let me dig it up and I'll make a PR.