transformers
transformers copied to clipboard
`ConditionalDetrImageProcessor` still accepts the deprecated parameter `max_size`
At the __init__ method of ConditionalDetrImageProcessor file, we still accept the deprecated parameter, max_size:
if "max_size" in kwargs:
logger.warning_once(
"The `max_size` parameter is deprecated and will be removed in v4.26. "
"Please specify in `size['longest_edge'] instead`.",
)
max_size = kwargs.pop("max_size")
else:
max_size = None if size is None else 1333
But the version of transformers is already 4.52.0.dev0 (from src/transformers/__init__.py).
ConditionalDetrImageProcessorFast also suffers the same issue.
We should remove the parameter, no?
cc: @amyeroberts, @qubvel
Seems like it's also the case for models like Deformable DETR, Grounding DINO, RT-DETR YOLOS: https://github.com/search?q=repo%3Ahuggingface%2Ftransformers+%22will+be+removed+in+v4.26%22&type=code. Would be great to remove it for all
@NielsRogge @arjunaskykok that's indeed an annoying BC issue.
The problem is that we still have very popular models which have max_size in configs on the Hub, see this and this comments in the related PR.
PR's are opened on the Hub to update them, but no reaction for most of them so far.
Hi is there any part which I can contribute too which is currently not under way. Would love tot work on it thanks.
This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.
Please note that issues that do not follow the contributing guidelines are likely to be ignored.
contributions are welcomed for this?
Contributions are welcome! However we have to verify there are no models on the hub with max_size parameter. You would have to use the huggingface_hub library to fetch the corresponding preprocessing configs filtered by conditional_detr model type. Let me know if you have any questions!
Hi @Rahul7-77, do you still plan to work on this? If not, I’d be happy to pick it up.
Hi @Rahul7-77 , I'd like to take this issue. I'll check hub configs and proceed with removal if safe.
Hi @qubvel, I've looked into this issue and attempted to filter models using the following code:
from huggingface_hub import list_models from huggingface_hub import ModelFilter
models = list_models(filter="conditional_detr"))
for model in models: print(model.modelId)
This displays a list of models. Do I need to go through them one by one to check whether they use the max_size parameter, or is there a more efficient way to do this?
Hey @qubvel @marcndo @NielsRogge 👋
I’ve written a small script to scan all models tagged with "conditional-detr" on the Hub and check whether any of them still use the deprecated max_size parameter in their preprocessor_config.json.
Here’s the script (feel free to cross-check it):
from huggingface_hub import HfApi, hf_hub_download
import json
# Initialize the Hugging Face API
api = HfApi()
# Search for models with 'conditional_detr' in their name
models = api.list_models(filter=['conditional_detr'])
model_counter = 0
models_with_max_size_in_config = []
models_with_error = []
models_with_no_config = []
for model in models:
model_counter += 1
model_id = model.modelId
try:
file = api.get_paths_info(repo_id=model_id, paths=["preprocessor_config.json"])
if file:
file_data = hf_hub_download(repo_id=model_id, filename="preprocessor_config.json")
with open(file_data, 'r') as f:
config = json.load(f)
if config.get("max_size") is not None:
models_with_max_size_in_config.append(model_id)
print(f"Model {model_id} has max_size in preprocessor_config.json: {config['max_size']}")
else:
models_with_no_config.append(model_id)
print(f"Model {model_id} does not have preprocessor_config.json.")
except Exception as e:
print(f"Error retrieving files for model {model_id}: {e}")
models_with_error.append(model_id)
continue
print(f"Total models found: {model_counter}")
print(f"Models with max_size in preprocessor_config.json: {len(models_with_max_size_in_config)}")
print(f"Models with errors: {len(models_with_error)}")
print(f"Models without preprocessor_config.json: {len(models_with_no_config)}")
# dump results to a json file
results = {
"total_models": model_counter,
"models_with_max_size_in_config": models_with_max_size_in_config,
"models_with_error": models_with_error,
"models_without_config": models_with_no_config
}
with open('model_check_results.json', 'w') as f:
json.dump(results, f, indent=4)
With the following output:
Total models found: 201
Models with max_size in preprocessor_config.json: 1
Models with errors: 3
Models without preprocessor_config.json: 13
{
"total_models": 201,
"models_with_max_size_in_config": [
"Omnifact/conditional-detr-resnet-101-dc5"
],
"models_with_error": [ ... gated models ... ],
"models_without_config": [ ... 13 repos ... ]
}
So, just a couple of questions here to double check.
- Is there any other config file we should check (besides
preprocessor_config.json) wheremax_sizecould still appear? - The only model using it is Omnifact/conditional-detr-resnet-101-dc5 – should I open a PR or issue on their repo to update this?
- The 3 error cases are gated/private models. Can we safely exclude them from this cleanup, or do they need follow-up?
Once confirmed, I think we’re good to proceed with updating ConditionalDetrImageProcessor.
Happy to repeat the process for other model types as well!
Thanks for iterating on it @druvdub, for the model "Omnifact/conditional-detr-resnet-101-dc5" - PR is already opened to replace max_size and the model seems not to be super popular, so we can safely remove max_size for the conditional detr model.
- https://huggingface.co/Omnifact/conditional-detr-resnet-101-dc5/discussions/2
Feel free to open a PR if you have bandwidth
Hi, I would love to take this up. I will open a PR soon unless anyone else is actively working on it.
Hey @kartickkt I am currently working on it, I am currently doing it for conditional_detr but am happy for you to pick up one of the other models check for them as well.
@qubvel just had a two-part question.
# Copied from transformers.models.detr.image_processing_detr.DetrImageProcessor.from_dict with Detr->ConditionalDetr
def from_dict(cls, image_processor_dict: dict[str, Any], **kwargs):
"""
Overrides the `from_dict` method from the base class to make sure parameters are updated if image processor is
created using from_dict and kwargs e.g. `ConditionalDetrImageProcessor.from_pretrained(checkpoint, size=600,
max_size=800)`
"""
image_processor_dict = image_processor_dict.copy()
if "max_size" in kwargs:
image_processor_dict["max_size"] = kwargs.pop("max_size")
- Should the
max_sizechecks be removed from other functions like this as well? I did remove it and updated the docstring to instead use size? but would I need some additional code to handle that? - Since, I am only making changes from
conditional_detrright now, I do get some errors when runningmake repo-consistencybecause of the# Copied from transformers.models.detr.image_processing_detr.DetrImageProcessor.from_dict with Detr->ConditionalDetr. How should I deal with this? I mean I can remove it but if and once the detr gets updated, these changes will be consistent again. Or can I just ignore these errors?
Update: Just made a Draft PR to clarify these things. Will check if any tests are needed and any updates required for the docs
Hey @druvdub, I will take up YOLOS. Will open a PR soon.
Hi @qubvel @druvdub, I have opened a PR removing max_size support from the YOLOS image processor:
This aligns with the same deprecation discussed here and ensures YOLOS processors conform to the updated API spec.
Happy to contribute further if needed!
Hi, I’m new and would like to work on this issue. Could you please assign it to me ?
Hi, I’m interested in contributing here. I’d be happy to take care of updating docs/tutorials/examples and ensuring consistency across all DETR-family processors.
cc @yonigozlan is this issue still relevant to work on?
Hi, I’d like to take this up as a first contribution. Can I work on it?
Hi! I’d like to work on this issue. Could you please assign it to me?