Skip to content

Save weights to custom folder of user 's choice #130

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

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

valavanisleonidas
Copy link
Contributor

@valavanisleonidas valavanisleonidas commented Apr 8, 2025

Description

This PR enhances the model downloading utility by allowing the user to choose any custom path to save the weights of the model. The destination path’s parent directories are created before saving the file.

The PR introduces one more config variable pretrain_save_file:Optional[str] = 'model.pth' which is used to select the path to save the file. it can be models/model.pth or just detr-large.pth for example. The variable pretrain_weights is used to select the model to use and has 3 values from the HOSTED_MODELS dict

rfdetr_base,
rfdetr_base2,
rfdetr_large

Type of change

  • New feature (non-breaking change which adds functionality)

How has this change been tested, please provide a testcase or example of how you tested the change?

Manually tested by setting pretrain_weights to a custom path and verifying:

  • All required directories were created if missing
  • File was successfully downloaded and saved
  • The logic works both when redownload is True and when the file does not exist

I have tested the following scenarios with comments on expected behavior

# downloads large model at `model.pth`
model = RFDETRLarge()
# crashes with error `Unknown model type`
model = RFDETRLarge(pretrain_save_file="",pretrain_weights="asfd")
# downloads as file models, unwanted behavior but it lets download
model = RFDETRLarge(pretrain_save_file="models",pretrain_weights="rfdetr_large")
# download model at `models/model.pth`
model = RFDETRLarge(pretrain_save_file="models/model.pth",pretrain_weights="rfdetr_large")

# also checked when fail to find path and has to redownload

Any specific deployment considerations

Docs

  • Docs updated? What were the changes:

@SkalskiP
Copy link
Collaborator

SkalskiP commented Apr 9, 2025

Hi @valavanisleonidas 👋🏻 Thank you so much for your interest in RF-DETR and for submitting this PR!

We won’t be merging this PR in its current form, and here’s why:

  • No other major library handles this in quite the same way. For example, libraries like Transformers (Hugging Face) allow customization via a cache_dir argument or environment variables like HF_HOME. Ultralytics, on the other hand, doesn’t support custom paths at all and sticks to a default location.
  • Currently, pretrain_weights should either be a valid path to existing weights on disk or one of the predefined model IDs for downloading to a local default location. In the future, we might extend it to support valid model IDs from the Roboflow platform as well.
  • If we were to enable customization of the model cache directory, it would be better implemented through an additional argument like cache_directory or an environment variable such as RF_HOME, aligning with established practices and offering a cleaner, more consistent user experience.

Thanks again for your contribution! Let me know if you’d like to discuss this further or explore alternative approaches.

@valavanisleonidas
Copy link
Contributor Author

valavanisleonidas commented Apr 9, 2025

Hello @SkalskiP,

thank you for your response. I was trying to implement something with minimal changes to the code. To be honest I took the idea from Ultralytics in which you can do YOLO("models/yolo12l.pt") and it downloads the file in folder models. The path you input to Yolo() is valid as long as you put the correct model name (yolo12l.pt etc).

Another idea is to have two properties one for selecting the model to download rf-detr-base , rf-detr-base2 and rf-detr-large and one for the path to download it to.

I just believe it's really useful to be able to select where to save the model if it is not in a .cache known folder somewhere because depending on the file you run, the folder level in the project structure you are in, you need to change the path or re-download it every time

@SkalskiP
Copy link
Collaborator

SkalskiP commented Apr 9, 2025

Yes, I believe the responsibility should be split between the two arguments.

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

Successfully merging this pull request may close these issues.

2 participants