fixing name position_embeddings to object_queries

[Lorenzobattistela]

What does this PR do?

This PR refers to #19833 , and it just update some variables/docstrings names. Quoting the Issue, the paper mentions that the position_embeddings argument of the cross-attention layer are these input embeddings called object queries. And the key_value_position_embeddings is refered to as spatial_position_embeddings.

Reopening PR #23091

This PR is limited to DETR model.

Notes

This is my first contribution, so I'm happy to adjust anything in this PR. I ran all tests and style, and it went all, except for one:
make fixup. I got the following output:

Reading the output, I assume it is about other file using classes in modeling_detr. I'll wait for updates. I will also wait for review for doc updating or more guidance.

Fixes # (issue)

Before submitting

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

Who can review?

@NielsRogge
@amyeroberts

[amyeroberts]

@Lorenzobattistela For the repo consistency and quality checks, you'll need to run make fix-copies and then make style and push any changes made

[Lorenzobattistela]

@amyeroberts Done, just updated with the changes for repo consistency and quality. I don't know why, but testing pipelines and torch tests are failling within the installation step (but I did not changed anything related to it), and the test_worflow also failed just for torch. I'll wait for next instructions. Thanks!

[amyeroberts]

@Lorenzobattistela hmmmm, interesting. Could you try rebasing on main?

Some of the tests are failing because of the changes in this PR: https://app.circleci.com/pipelines/github/huggingface/transformers/67974/workflows/6a69bd9f-d35a-4964-868b-14fdd921d813/jobs/850696

Once these are resolved, ping me again and I can review :)

[Lorenzobattistela]

@amyeroberts Sorry for bothering, but I'm having a hard time with the circleCi testing. So, I'm having problems on repo consistency (as you mentioned before), but if I do run the script make fix-copies it change other models files (3 of them), and I think this would be scaping the Issue scope.

About the tests, I'm getting the following output:

FAILED tests/models/detr/test_modeling_detr.py::DetrModelTest::test_attention_outputs - RuntimeError: The size of tensor a (12) must match the size of tensor b (49) at non-singleton dimension 1
FAILED tests/models/detr/test_modeling_detr.py::DetrModelTest::test_determinism - RuntimeError: The size of tensor a (12) must match the size of tensor b (49) at non-singleton dimension 1
FAILED tests/models/detr/test_modeling_detr.py::DetrModelTest::test_detr_model - RuntimeError: The size of tensor a (12) must match the size of tensor b (49) at non-singleton dimension 1
FAILED tests/models/detr/test_modeling_detr.py::DetrModelTest::test_detr_no_timm_backbone - RuntimeError: The size of tensor a (12) must match the size of tensor b (49) at non-singleton dimension 1
FAILED tests/models/detr/test_modeling_detr.py::DetrModelTest::test_detr_object_detection_head_model - RuntimeError: The size of tensor a (12) must match the size of tensor b (49) at non-singleton dimension 1
FAILED tests/models/detr/test_modeling_detr.py::DetrModelTest::test_different_timm_backbone - RuntimeError: The size of tensor a (12) must match the size of tensor b (49) at non-singleton dimension 1
FAILED tests/models/detr/test_modeling_detr.py::DetrModelTest::test_feed_forward_chunking - RuntimeError: The size of tensor a (12) must match the size of tensor b (49) at non-singleton dimension 1
FAILED tests/models/detr/test_modeling_detr.py::DetrModelTest::test_greyscale_images - RuntimeError: The size of tensor a (12) must match the size of tensor b (49) at non-singleton dimension 1
FAILED tests/models/detr/test_modeling_detr.py::DetrModelTest::test_hidden_states_output - RuntimeError: The size of tensor a (12) must match the size of tensor b (49) at non-singleton dimension 1
FAILED tests/models/detr/test_modeling_detr.py::DetrModelTest::test_retain_grad_hidden_states_attentions - RuntimeError: The size of tensor a (12) must match the size of tensor b (49) at non-singleton dimension 1
FAILED tests/models/detr/test_modeling_detr.py::DetrModelTest::test_save_load - RuntimeError: The size of tensor a (12) must match the size of tensor b (49) at non-singleton dimension 1
FAILED tests/models/detr/test_modeling_detr.py::DetrModelTest::test_training - RuntimeError: The size of tensor a (12) must match the size of tensor b (49) at non-singleton dimension 1
=== 12 failed, 1419 passed, 2461 skipped, 144 warnings in 163.96s (0:02:43) 

The funny thing is that I did not changed anything related to tensor sizes, since it was just naming convention

[amyeroberts]

@Lorenzobattistela No worries, you're not bothering at all :)

if I do run the script make fix-copies it change other models files (3 of them), and I think this would be scaping the Issue scope.

It's OK, we do want the changes made by make fix-copies included in this PR. make fix-copies makes sure that changes to the code are propagated across to all part of the codebase where the logic has been copied without the tedium or riskiness of doing it manually. This allows us to keep the one file per model pattern in the library.

The funny thing is that I did not changed anything related to tensor sizes, since it was just naming convention

Hmmm, funny. It might be that there's a var somewhere still needing it's name changed, or it could be how the model's being called in the tests. I'd suggest picking just one test and run that with the debugger to find where the issue is coming from i.e.

pytest tests/models/detr/test_modeling_detr.py::DetrModelTest::test_attention_outputs --pdb

and comparing the tensor shapes with and without the changes in this PR to track where they're coming from.

[Lorenzobattistela]

@amyeroberts Got it working! It was a problem with make fix-copies, so some other files had to change to keep consistency and pass up the tests. Now it's all set!

[amyeroberts]

Thanks for your work adding this!

Main comment is about making sure the public methods are backwards compatible. Other than that the PR is looking v. nice and clean 🤗

[HuggingFaceDocBuilderDev]

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint.

[Lorenzobattistela]

@amyeroberts finished doing what was discussed. I think we can also think about refactoring and add it as a function, something like check_kwargs() , idk.
Because it was mostly duplicated accross all files. What do you think about it?

Weird, the error on CI has nothing to do with the files changed, its on other model

[amyeroberts]

Thanks for all your work on this and iterating. Looks great!