transformers icon indicating copy to clipboard operation
transformers copied to clipboard

`ConditionalDetrImageProcessor` still accepts the deprecated parameter `max_size`

Open arjunaskykok opened this issue 7 months ago • 10 comments

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

arjunaskykok avatar May 03 '25 14:05 arjunaskykok

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 avatar May 05 '25 07:05 NielsRogge

@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.

qubvel avatar May 05 '25 18:05 qubvel

Hi is there any part which I can contribute too which is currently not under way. Would love tot work on it thanks.

demoncoder-crypto avatar May 05 '25 23:05 demoncoder-crypto

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.

github-actions[bot] avatar Jun 03 '25 08:06 github-actions[bot]

contributions are welcomed for this?

Rahul7-77 avatar Jun 04 '25 03:06 Rahul7-77

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!

qubvel avatar Jun 06 '25 10:06 qubvel

Hi @Rahul7-77, do you still plan to work on this? If not, I’d be happy to pick it up.

marcndo avatar Jun 07 '25 17:06 marcndo

Hi @Rahul7-77 , I'd like to take this issue. I'll check hub configs and proceed with removal if safe.

sarashahin avatar Jun 07 '25 20:06 sarashahin

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?

marcndo avatar Jun 11 '25 19:06 marcndo

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.

  1. Is there any other config file we should check (besides preprocessor_config.json) where max_size could still appear?
  2. 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?
  3. 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!

druvdub avatar Jun 15 '25 02:06 druvdub

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

qubvel avatar Jun 17 '25 12:06 qubvel

Hi, I would love to take this up. I will open a PR soon unless anyone else is actively working on it.

kartickkt avatar Jun 18 '25 16:06 kartickkt

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")
  1. Should the max_size checks 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?
  2. Since, I am only making changes from conditional_detr right now, I do get some errors when running make repo-consistency because 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

druvdub avatar Jun 18 '25 17:06 druvdub

Hey @druvdub, I will take up YOLOS. Will open a PR soon.

kartickkt avatar Jun 18 '25 21:06 kartickkt

Hi @qubvel @druvdub, I have opened a PR removing max_size support from the YOLOS image processor:

#38923

This aligns with the same deprecation discussed here and ensures YOLOS processors conform to the updated API spec.

Happy to contribute further if needed!

kartickkt avatar Jun 19 '25 22:06 kartickkt

Hi, I’m new and would like to work on this issue. Could you please assign it to me ?

Abhijais4896 avatar Sep 18 '25 15:09 Abhijais4896

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.

ahmedvictor507 avatar Sep 27 '25 04:09 ahmedvictor507

cc @yonigozlan is this issue still relevant to work on?

NielsRogge avatar Sep 29 '25 08:09 NielsRogge

Hi, I’d like to take this up as a first contribution. Can I work on it?

sonianuj287 avatar Oct 02 '25 10:10 sonianuj287

Hi! I’d like to work on this issue. Could you please assign it to me?

Raufnarejo505 avatar Oct 06 '25 09:10 Raufnarejo505