Skip to content

Fixed VitDet for non-squre Images - #35969

Merged
qubvel merged 13 commits into
huggingface:mainfrom
cjfghk5697:issue/vitDet-nonsquare
Feb 25, 2025
Merged

Fixed VitDet for non-squre Images#35969
qubvel merged 13 commits into
huggingface:mainfrom
cjfghk5697:issue/vitDet-nonsquare

Conversation

@cjfghk5697

@cjfghk5697 cjfghk5697 commented Jan 30, 2025

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #35280 (issue)

The VitDet model doesn't support tuple-type inputs for image_size and patch_size. I’ve fixed it so that tuples can now be used.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • [ x ] Did you read the contributor guideline,
    Pull Request section?
  • [ x ] Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • [ x ] Did you write any new necessary tests?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@qubvel @amyeroberts

@cjfghk5697 cjfghk5697 changed the title Issue/vit det nonsquare Fixed VitDet for non-squre Images Jan 30, 2025
@qubvel

qubvel commented Jan 31, 2025

Copy link
Copy Markdown
Contributor

Hey @cjfghk5697! Thanks for opening a PR 🤗 Feel free to ping me when it's ready for review

@cjfghk5697

Copy link
Copy Markdown
Contributor Author

@qubvel Thanks for your response! Would you like me to provide a mock code to check the tuple case?

@qubvel qubvel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Can you please add a test to tests/models/test_modeling_vitdet.py which allows passing non-square image through the model

Comment on lines +455 to +462
if isinstance(config.image_size, tuple) and isinstance(config.patch_size, tuple):
input_size = tuple(i // p for i, p in zip(config.image_size, config.patch_size))
elif isinstance(config.image_size, tuple) and not isinstance(config.patch_size, tuple):
input_size = tuple(i // config.patch_size for i in config.image_size)
elif not isinstance(config.image_size, tuple) and isinstance(config.patch_size, tuple):
input_size = tuple(config.image_size // p for p in config.patch_size)
else:
input_size = config.image_size // config.patch_size

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if isinstance(config.image_size, tuple) and isinstance(config.patch_size, tuple):
input_size = tuple(i // p for i, p in zip(config.image_size, config.patch_size))
elif isinstance(config.image_size, tuple) and not isinstance(config.patch_size, tuple):
input_size = tuple(i // config.patch_size for i in config.image_size)
elif not isinstance(config.image_size, tuple) and isinstance(config.patch_size, tuple):
input_size = tuple(config.image_size // p for p in config.patch_size)
else:
input_size = config.image_size // config.patch_size
image_size = config.image_size
image_size = image_size if isinstance(image_size, (list, tuple)) else (image_size, image_size)
patch_size = config.patch_size
patch_size = patch_size if isinstance(patch_size, (list, tuple)) else (patch_size, patch_size)
input_size = (config.image_size[0] // config.patch_size[0], config.image_size[1] // config.patch_size[1])

@cjfghk5697

Copy link
Copy Markdown
Contributor Author

@qubvel, thank you for reviewing it🤗. I've finished adding the test code. Could you please check it again?

@qubvel qubvel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, to ensure CI is green, please run make modified_only_fixup to reformat the code 🤗

@cjfghk5697

Copy link
Copy Markdown
Contributor Author

@qubvel I’ve finished running make modified_only_fixup—thanks for your help!

@qubvel

qubvel commented Feb 6, 2025

Copy link
Copy Markdown
Contributor

hm... something goes wrong, probably you have older/newer version of ruff? Can you please check and update accordingly?

@cjfghk5697

Copy link
Copy Markdown
Contributor Author

hm... something goes wrong, probably you have older/newer version of ruff? Can you please check and update accordingly?

I tried running it with ruff==0.5.1, but the results look really strange. Could you let me know which version you're using?

@qubvel

qubvel commented Feb 6, 2025

Copy link
Copy Markdown
Contributor

It's ruff 0.5.1 in my env, it might be that you are running with no pyproject.toml configuration?

@qubvel

qubvel commented Feb 6, 2025

Copy link
Copy Markdown
Contributor

It's better to delete 3 last commits, apply a modified fixup, and then force push

@cjfghk5697
cjfghk5697 force-pushed the issue/vitDet-nonsquare branch from fb2998f to 713e610 Compare February 7, 2025 05:52
@cjfghk5697

Copy link
Copy Markdown
Contributor Author

@qubvel Sorry about that! I created a new environment and deleted some commits, which ended up modifying the fixup. Let me know if anything needs to be adjusted.

@cjfghk5697

Copy link
Copy Markdown
Contributor Author

@qubvel Can you review it again?

@qubvel

qubvel commented Feb 17, 2025

Copy link
Copy Markdown
Contributor

Hey @cjfghk5697 can you please fix quality issues? most probably make modified_only_fixup will resolve the issue

@cjfghk5697

Copy link
Copy Markdown
Contributor Author

@qubvel Thank you for your help. I checked and discovered that there was an issue with my environment. I've applied the fix!🤗

@qubvel

qubvel commented Feb 19, 2025

Copy link
Copy Markdown
Contributor

Hey @cjfghk5697, with the recent modification we have a bunch of tests failed, can you please check what should be fixed?

FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_attention_outputs - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_backbone - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_batching_equivalence - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_can_use_safetensors - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_correct_missing_keys - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_determinism - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_from_pretrained_no_checkpoint - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_gradient_checkpointing_backward_compatibility - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_gradient_checkpointing_enable_disable - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_hidden_states_output - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_initialization - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_inputs_embeds_matches_input_ids - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_load_save_without_tied_weights - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_model - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_model_get_set_embeddings - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_model_is_small - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_model_outputs_equivalence - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_model_weights_reload_no_missing_tied_weights - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_peft_gradient_checkpointing_enable_disable - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_retain_grad_hidden_states_attentions - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_save_load - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_save_load_fast_init_from_base - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_save_load_fast_init_to_base - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_save_load_keys_to_ignore_on_save - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_tie_model_weights - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_tied_weights_keys - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetModelTest::test_torch_save_load - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetBackboneTest::test_backbone_common_attributes - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetBackboneTest::test_backbone_outputs - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetBackboneTest::test_backbone_stage_selection - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetBackboneTest::test_channels - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetBackboneTest::test_create_from_modified_config - TypeError: 'int' object is not subscriptable
FAILED tests/models/vitdet/test_modeling_vitdet.py::VitDetBackboneTest::test_forward_signature - TypeError: 'int' object is not subscriptable

Comment thread src/transformers/models/vitdet/modeling_vitdet.py Outdated
@qubvel

qubvel commented Feb 19, 2025

Copy link
Copy Markdown
Contributor

I think I got why tests failed, I hope you don't mind I've pushed the fix 🤗

@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@qubvel
qubvel requested a review from yonigozlan February 19, 2025 12:08
@qubvel

qubvel commented Feb 19, 2025

Copy link
Copy Markdown
Contributor

cc @yonigozlan can you take a look

@cjfghk5697

Copy link
Copy Markdown
Contributor Author

@qubvel I sincerely appreciate your help in successfully wrapping up this task🤗

@yonigozlan yonigozlan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me @cjfghk5697 ! Thanks for making the model more flexible 🤗


def test_non_square_image(self):
non_square_image_size = (32, 40)
patch_size = (2, 2)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know too much about this model, but do we also want to test a non-square patch size?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, maybe a bit out of scope for this PR, but should also work

@qubvel
qubvel merged commit 9ebfda3 into huggingface:main Feb 25, 2025
@qubvel qubvel added the Vision label Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vision models don't work for non-square object

5 participants