Implementation of SuperPoint and AutoModelForInterestPointDescription
Author: sbucaille
Pull request: yes

# What does this PR do?

This PR implements SuperPoint, one of the few models that generate keypoints and descriptors given an image, as discussed in [this previous pull request](https://github.com/huggingface/transformers/pull/25697)
The goal is to implement this model and a new type of AutoModel : `AutoModelForInterestPointDescription` (name to be discussed).


## Who can review?

@amyeroberts @ArthurZucker 

## TODO's

- [x] Implement SuperPointConfig and SuperPointModel as PretrainedConfig and PretrainedModel
- [x] Generate a conversion script for the original weights
- [x] Implement the new `AutoModelForInterestPointDescription` mapping
- [x] Test the model
- [x] Write documentation



Comments:
@amyeroberts I have some questions about the implementation and AutoModel classes.

First of all, I try to follow as much as possible the patterns I see in other model implementations (resnet or convnextv2 for example), but unlike these models, SuperPoint really only have one function or "mode" here, just to output the keypoints, their scores and descriptors. This is why I only implemented SuperPoint as a `SuperPointModelForInterestPointDescription`, so there is no `SuperPointModel` anymore, does that seem ok ?

Then I added this `SuperPointModelForInterestPointDescription` class in a new mapping dictionary in the modeling_auto file and added the appropriate AutoModel class for this. But is this kind of changes usually the output of an automated script for model registration or adding it by hand is appropriate ?

Finally, 

> In that PR, we can also add a mapping AutoModelForInterestPointDescription, which we define as taking two images and returning interest keypoints and their descriptions.

Apart from adding the AutoModelForInterestPointDescription, I couldn't find how to define such inputs and outputs, is it a new pipeline I should define or something else ?

Hi @sbucaille, 

This is a bit of a special case. For other models which only perform a single task, what we normally do is just have `XxxModel`. I'd suggest doing this. We can still add `AutoModelForInterestPointDescription` and have `SuperPointModel`  loaded by it

@sbucaille From next week, I'll be off for a few weeks. If you have vision-specific questions, please ping @rafaelpadilla; for implementation questions @ArthurZucker. 

Hi @ArthurZucker, I added the SuperPointImageProcessor as part of the code because SuperPoint requires a grayscale image as input. But when I added the tests I have the test_call_pil which fails and give me a very weird when it reaches these lines 
```
tests/models/superpoint/test_image_processing_superpoint.py:43: in prepare_image_inputs
    return prepare_image_inputs(
tests/test_image_processing_common.py:64: in prepare_image_inputs
    image_inputs = [Image.fromarray(np.moveaxis(image, 0, -1)) for image in image_inputs]
tests/test_image_processing_common.py:64: in <listcomp>
    image_inputs = [Image.fromarray(np.moveaxis(image, 0, -1)) for image in image_inputs]
```
with the following error : 
```
            except KeyError as e:
                msg = "Cannot handle this data type: %s, %s" % typekey
>               raise TypeError(msg) from e
E               TypeError: Cannot handle this data type: (1, 1, 1), |u1
```

Not sure what causes the problem as I tried to compare with tests made with the ConvNextImageProcessor which does not raise any error.

Anyway, I continue on the implementation, let me know if I'm missing anything. I'll write the documentation for all the code I've previously pushed.






> Hi @ArthurZucker, I added the SuperPointImageProcessor as part of the code because SuperPoint requires a grayscale image as input. But when I added the tests I have the test_call_pil which fails and give me a very weird when it reaches these lines
> 
> ```
> tests/models/superpoint/test_image_processing_superpoint.py:43: in prepare_image_inputs
>     return prepare_image_inputs(
> tests/test_image_processing_common.py:64: in prepare_image_inputs
>     image_inputs = [Image.fromarray(np.moveaxis(image, 0, -1)) for image in image_inputs]
> tests/test_image_processing_common.py:64: in <listcomp>
>     image_inputs = [Image.fromarray(np.moveaxis(image, 0, -1)) for image in image_inputs]
> ```
> 
> with the following error :
> 
> ```
>             except KeyError as e:
>                 msg = "Cannot handle this data type: %s, %s" % typekey
> >               raise TypeError(msg) from e
> E               TypeError: Cannot handle this data type: (1, 1, 1), |u1
> ```
> 
> Not sure what causes the problem as I tried to compare with tests made with the ConvNextImageProcessor which does not raise any error.
> 
> Anyway, I continue on the implementation, let me know if I'm missing anything. I'll write the documentation for all the code I've previously pushed.

Hi @sbucaille, 🙂

A quick help with that issue: 

I see that your processing converts all images to grayscale ([here](https://github.com/huggingface/transformers/blob/28e7d59ab920741ac7f52923f39d1a9680ca3911/src/transformers/models/superpoint/image_processing_superpoint.py#L209C29-L209C49)) and tests are failing [here](https://github.com/huggingface/transformers/blob/000e52aec8850d3fe2f360adc6fd256e5b47fe4c/tests/test_image_processing_common.py#L64C24-L64C94).

The root cause is that the `convert_to_grayscale` function returns a 1-channel image (luminance) as [here](https://github.com/huggingface/transformers/blob/28e7d59ab920741ac7f52923f39d1a9680ca3911/src/transformers/image_transforms.py#L787C5-L788C1). So, when it is later converted to a numpy array, it will turn to be a 1-channel image, making the test fail.

This has been discussed in PR #25767 and is not fully solved yet.

A quick solution for this issue in your code may be possible. See that a 1 single channel image is definetely grayscale, but if the 3 channels in an RGB image are equal (R==G and G==B), the image is also noted as grayscale. So, if you replicate the channels of your 1-channel grayscale image [as in here](https://github.com/huggingface/transformers/blob/79f53921de5bd15ef7f4cdd83fd94d2b4cb8f63b/src/transformers/image_utils.py#L178), this issue can be solved. However, SuperPoint would need to be adapted for that -> you would only need to consider one of the RGB channels, as they are equal.



Hi @rafaelpadilla and @ArthurZucker  ,
Thanks @rafaelpadilla for the heads up, I adapted SuperPointModel and SuperPointImageProcessor to cover this issue. SuperPointImageProcessor now generates a 3-channel grayscaled image from a given image as input and SuperPointModel extracts one of the channels to perform the forward method. Although it may be necessary to change that in the future when 1-channel images will be supported (if it is planned to).
I added docs, as well as remaining integration tests for the AutoModelForInterestPointDescription.
I think the implementation is complete.

@ArthurZucker, please let me know what I'm missing in the implementation ! :slightly_smiling_face: 
Although I have some questions : 
- What should I do with the SuperPointModel and SuperPointModelForInterestPointDescription as both are basically the same, should I only keep the latter one ?
- Regarding docs, there is a mention of `expected_output` is `@add_code_sample_docstrings`. I decided not to provide such information since output is dynamic, depending on the number of keypoints found. Should I keep it like that or there is a way to provide a "dynamic shape" to this function ?
- Regarding tests, I have the `test_model_is_small` failing, what should I do about it ? And is `test_retain_grad_hidden_states_attentions` related to models that can be trained ? If so we should probably skip it since SuperPoint can't be trained, also it does not have attentions.

Hi,
When adding the docs to the code on this Saturday, I started thinking, maybe late, about the licence of SuperPoint and got an answer from an original contributor, Paul-Edouard Sarlin.
It turns out it can't be used for commercial use. I am not very familiar with legal stuff like these, but does it compromise this PR ? Or, from the HuggingFace perspective, adding the licence as in the original repo is sufficient ? I added it to the [model card](https://huggingface.co/stevenbucaille/superpoint/blob/main/LICENSE) anyway.

> Hi, When adding the docs to the code on this Saturday, I started thinking, maybe late, about the licence of SuperPoint and got an answer from an original contributor, Paul-Edouard Sarlin. It turns out it can't be used for commercial use. I am not very familiar with legal stuff like these, but does it compromise this PR ? Or, from the HuggingFace perspective, adding the licence as in the original repo is sufficient ? I added it to the [model card](https://huggingface.co/stevenbucaille/superpoint/blob/main/LICENSE) anyway.

Hi @sbucaille,

It seems that the original code is under MIT license. If it is the case, you just need to add the MIT license on the top of the files, as done in [graphormer](https://github.com/huggingface/transformers/blob/0ee45906845c8d58b9bd2df5acd90e09b00047ff/docs/source/en/model_doc/graphormer.md?plain=1#L3C1-L3C32) and [IDEFICS](https://github.com/huggingface/transformers/blob/0ee45906845c8d58b9bd2df5acd90e09b00047ff/src/transformers/models/idefics/perceiver.py#L1-L23).

The checkpoints seem to be under a non-commercial customized license. So, as you have already added the License [here](https://huggingface.co/stevenbucaille/superpoint/blob/main/LICENSE), you just need to set `inference: false` in the card as done in [owlvit-large-patch14](https://huggingface.co/google/owlvit-large-patch14/blob/main/README.md) and [musicgen-large](https://huggingface.co/facebook/musicgen-large/blob/main/README.md). 

I figured out the two remaining tests I asked were not in the scope of SuperPoint. Ready for review :hugs: 

Hi @rafaelpadilla @ArthurZucker,
I was about to start working again on the other [pull request](https://github.com/huggingface/transformers/pull/25697) I made earlier about SuperGlue, which for reminder, uses SuperPoint as a keypoint detector, and uses these keypoints to match them. While I was trying things around I was thinking about the case where we have batched images.
SuperPoint, and as a consequence SuperGlue, does not support batched images, since the output for a given image is an arbitrary number of keypoints, which are different for every image, hence the resulting keypoint tensors can't be batched back together.
I added in the tests that the batch size needs to be one in order for the tests to succeed, but there is no batch verification made when multiple images are fed into `SuperPointModel`.
I have two suggestions to handle this problem :
- Raise an error as soon as SuperPoint detects that we fed `pixel_values` with a batch_size > 1
- Support image batching by decomposing the batched `pixel_values`, detecting the keypoints from one image at a time and compose back the keypoints. Again, two possibilities :
  - By changing `ImagePointDescriptionOutput` so that it returns a list of `torch.IntTensor` for `keypoints`, `scores` and `descriptors` which are individually of different shapes (which to be honest I don't really like)
  - By keeping `ImagePointDescriptionOutput` identical but make `keypoints`, `scores` and `descriptors` batched together with shape looking like : 
    - `[batch_size, SuperPointConfig.max_keypoints, 2]`
    - `[batch_size, SuperPointConfig.max_keypoints]`
    - `[batch_size, 256, SuperPointConfig.max_keypoints]` 
Or : 
    - `[batch_size, maximum_found_keypoints, 2]`
    - `[batch_size, maximum_found_keypoints]`
    - `[batch_size, 256, maximum_found_keypoints]` 
in case `SuperPointConfig.max_keypoints == -1` with `maximum_found_keypoints` being the maximum number of keypoints found for an image in the given batch. In this case, we would fill values generated by the broadcast with 0's or -1's which would need to be pruned away in a later stage, for example in an ImageMatching pipeline. Alternatively, we could add a 4th tensor which would be binary and tell what values are actual keypoint information or filled values.

Let me know what you think, maybe adding batching can be a future step and we can deal with single images for now by raising errors. 
I'm not very familiar with the existing implemented pipelines, but another suggestion may be the support of batching by adding the aforementioned suggestions in a pipeline, which would prevent us from adding this kind of behavior inside the models themselves. We could consider using the pipeline as the only way of using SuperPoint and other ModelForInterestPointDescription with multiple images batched together. Also, if we apply this behavior in the models themselves, potentially, we would need to support batching for all of them in the future, leading to repetitive code to be implemented.

I hope the suggestions are clear, as at some point I wrote them down as I was thinking, let me know what you think ! :slightly_smiling_face: 

Hi @sbucaille ,

Thank you for raising this discussion. 🙂

I think supporting batch > 1 is a good idea. So, the size of the `keypoints`, `scores` and `descriptors` in all images within the batch should match. For that, I would opt for:
* `keypoints` -> `[batch_size, SuperPointConfig.max_keypoints, 2]`
* `scores` -> `[batch_size, SuperPointConfig.max_keypoints]` or explicitly `[batch_size, SuperPointConfig.max_keypoints, 1]`
* `descriptors` -> `[batch_size, SuperPointConfig.max_keypoints, 256]`

I would make the last dimension to represent the size of the data itself.

Defining the shapes of these vectors with `SuperPointConfig.max_keypoints` instead of `maximum_found_keypoints` will keep consistency when other/different images are input with the same configuration.

I would also use a 4th mask vector made of `bool` (`mask -> [batch_size, SuperPointConfig.max_keypoints]` to inform if the information should be used for that particular image or not.


Hi @rafaelpadilla ,
Changes have been made to support batching. I added the logic of batching individual inferences inside the SuperPointModel itself. In the future, for example in the implementation of a model like DISK, the same logic could be extracted into a dedicated function where, given a list of keypoints, scores and descriptors, return a batched version of it along a mask tensor.
About the `SuperPointConfig.max_keypoints`, the batching logic do not use the config parameter since SuperPoint outputs are already "filtered" by this parameter in the forward method.

Let me know what should be done next, but in the meantime, I think it's ready to get reviewed.

Hi @sbucaille ,

Thank you! I will do the first review on your PR. :) 

Hi @rafaelpadilla ,
I resolved most of the comments you and left answers on unresolved ones. Gotta say I'm a bit ashamed of all the missing types I let through :laughing: 
I have a problem with the merging conflict of the index.md I had. At first I wanted to retrieve an updated index.md file so I synced my fork with the main branch. But for some reason now when I run `make repo-consistency`, I have the following error : 
```
ValueError: There was at least one problem when checking docstrings of public objects.
The following objects docstrings do not match their signature. Run `make fix-copies` to fix this.
- TFRegNetForImageClassification
- TFRegNetModel
make: *** [Makefile:46: repo-consistency] Error 1
```

which does not seem to be related to my code and maybe the result of my synchronization. I'll try tomorrow to "unsync" my fork and revert it to the previous state.

Anyway, thanks for the quick review !

Hi @rafaelpadilla , turns out I am not the only one having this error in `make repo-consistency` with the TFRegNetModel, (see #26643).
I pushed a last commit that makes all the tests regarding SuperPoint passed. The other tests not passing do not seem to be because of my code, unless I'm wrong.

Hi @sbucaille Are you also using Mac? Could you run `pip freeze` and copy-paste the results?

Are you able to run `pip install -U -e .[dev]` (please try this in a new fresh python env., if you want to do it).

> Hi @sbucaille Are you also using Mac? Could you run `pip freeze` and copy-paste the results?
> 
> Are you able to run `pip install -U -e .[dev]` (please try this in a new fresh python env., if you want to do it).

Hi @ydshieh ,
I posted this comment right after creating a new env from this exact same command, and I'm running it in Ubuntu. I believe this is related to the main branch.

Could you run `transformers-cli env` and copy paste the output.

Also, after the installation, run `pip freeze` and copy paste the output.

Thank you in advance!

```
❯ transformers-cli env
2023-10-12 20:01:13.177759: W tensorflow/compiler/tf2tensorrt/utils/py_utils.cc:38] TF-TRT Warning: Could not find TensorRT
WARNING:tensorflow:From /home/steven/PycharmProjects/transformers/src/transformers/commands/env.py:100: is_gpu_available (from tensorflow.python.framework.test_util) is deprecated and will be removed in a future version.
Instructions for updating:
Use `tf.config.list_physical_devices('GPU')` instead.
2023-10-12 20:01:14.165161: W tensorflow/core/common_runtime/gpu/gpu_device.cc:1960] Cannot dlopen some GPU libraries. Please make sure the missing libraries mentioned above are installed properly if you would like to use GPU. Follow the guide at https://www.tensorflow.org/install/gpu for how to download and setup the required libraries for your platform.
Skipping registering GPU devices...
No GPU/TPU found, falling back to CPU. (Set TF_CPP_MIN_LOG_LEVEL=0 and rerun for more info.)

Copy-and-paste the text below in your GitHub issue and FILL OUT the two last points.

- `transformers` version: 4.35.0.dev0
- Platform: Linux-5.19.0-46-generic-x86_64-with-glibc2.35
- Python version: 3.10.12
- Huggingface_hub version: 0.16.4
- Safetensors version: 0.3.2
- Accelerate version: 0.23.0
- Accelerate config: 	not found
- PyTorch version (GPU?): 2.0.1+cu117 (True)
- Tensorflow version (GPU?): 2.13.0 (False)
- Flax version (CPU?/GPU?/TPU?): 0.7.0 (cpu)
- Jax version: 0.4.13
- JaxLib version: 0.4.13
- Using GPU in script?: <fill in>
- Using distributed or parallel set-up in script?: <fill in>
```

```
❯ pip freeze
absl-py==2.0.0
accelerate==0.23.0
aiohttp==3.8.5
aiosignal==1.3.1
alembic==1.12.0
ansi2html==1.8.0
APScheduler==3.10.4
arrow==1.2.3
astunparse==1.6.3
async-timeout==4.0.3
attrs==23.1.0
audioread==3.0.0
av==9.2.0
Babel==2.12.1
backoff==1.11.1
beautifulsoup4==4.12.2
binaryornot==0.4.4
black==23.9.1
cachetools==5.3.1
certifi==2023.7.22
cffi==1.15.1
chardet==5.2.0
charset-normalizer==3.2.0
chex==0.1.7
click==8.1.7
clldutils==3.20.0
cmaes==0.10.0
cmake==3.27.5
codecarbon==1.2.0
colorama==0.4.6
colorlog==6.7.0
cookiecutter==1.7.3
csvw==3.1.3
dash==2.13.0
dash-bootstrap-components==1.5.0
dash-core-components==2.0.0
dash-html-components==2.0.0
dash-table==5.0.0
datasets==2.14.5
decorator==5.1.1
decord==0.6.0
dill==0.3.4
dlinfo==1.2.1
dm-tree==0.1.8
etils==1.5.0
evaluate==0.4.0
exceptiongroup==1.1.3
execnet==2.0.2
faiss-cpu==1.7.4
fastjsonschema==2.18.0
filelock==3.12.2
fire==0.5.0
Flask==2.2.5
flatbuffers==23.5.26
flax==0.7.0
frozenlist==1.4.0
fsspec==2023.6.0
fugashi==1.3.0
gast==0.4.0
gitdb==4.0.10
GitPython==3.1.18
google-auth==2.23.0
google-auth-oauthlib==1.0.0
google-pasta==0.2.0
gql==3.4.1
graphql-core==3.2.3
greenlet==2.0.2
grpcio==1.58.0
h5py==3.9.0
hf-doc-builder==0.4.0
huggingface-hub==0.16.4
hypothesis==6.87.0
idna==3.4
importlib-resources==6.1.0
iniconfig==2.0.0
ipadic==1.0.0
isodate==0.6.1
isort==5.12.0
itsdangerous==2.1.2
jax==0.4.13
jaxlib==0.4.13
Jinja2==3.1.2
jinja2-time==0.2.0
joblib==1.3.2
jsonschema==4.19.1
jsonschema-specifications==2023.7.1
jupyter_core==5.3.1
kenlm==0.2.0
keras==2.13.1
keras-core==0.1.7
keras-nlp==0.6.2
language-tags==1.2.0
lazy_loader==0.3
libclang==16.0.6
librosa==0.10.1
lit==17.0.1
llvmlite==0.41.0
lxml==4.9.3
Mako==1.2.4
Markdown==3.4.4
markdown-it-py==3.0.0
MarkupSafe==2.1.3
mdurl==0.1.2
ml-dtypes==0.3.1
mpmath==1.3.0
msgpack==1.0.6
multidict==6.0.4
multiprocess==0.70.12.2
mypy-extensions==1.0.0
namex==0.0.7
nbformat==5.9.2
nest-asyncio==1.5.8
networkx==3.1
nltk==3.8.1
numba==0.58.0
numpy==1.24.3
nvidia-cublas-cu11==11.10.3.66
nvidia-cuda-cupti-cu11==11.7.101
nvidia-cuda-nvrtc-cu11==11.7.99
nvidia-cuda-runtime-cu11==11.7.99
nvidia-cudnn-cu11==8.5.0.96
nvidia-cufft-cu11==10.9.0.58
nvidia-curand-cu11==10.2.10.91
nvidia-cusolver-cu11==11.4.0.1
nvidia-cusparse-cu11==11.7.4.91
nvidia-nccl-cu11==2.14.3
nvidia-nvtx-cu11==11.7.91
oauthlib==3.2.2
onnx==1.14.1
onnxconverter-common==1.13.0
opt-einsum==3.3.0
optax==0.1.4
optuna==3.3.0
orbax-checkpoint==0.4.0
packaging==23.1
pandas==2.1.1
parameterized==0.9.0
pathspec==0.11.2
phonemizer==3.2.1
Pillow==9.5.0
plac==1.4.0
platformdirs==3.10.0
plotly==5.17.0
pluggy==1.3.0
pooch==1.7.0
portalocker==2.0.0
poyo==0.5.0
protobuf==3.20.3
psutil==5.9.5
py-cpuinfo==9.0.0
pyarrow==13.0.0
pyasn1==0.5.0
pyasn1-modules==0.3.0
pycparser==2.21
pyctcdecode==0.5.0
Pygments==2.16.1
pygtrie==2.5.0
pylatexenc==2.10
pynvml==11.5.0
pyparsing==3.1.1
pypng==0.20220715.0
pytest==7.4.2
pytest-timeout==2.1.0
pytest-xdist==3.3.1
python-dateutil==2.8.2
python-slugify==8.0.1
pytz==2023.3.post1
PyYAML==6.0.1
ray==2.7.0
rdflib==7.0.0
referencing==0.30.2
regex==2023.8.8
requests==2.31.0
requests-oauthlib==1.3.1
requests-toolbelt==0.10.1
responses==0.18.0
retrying==1.3.4
rfc3986==1.5.0
rhoknp==1.3.0
rich==13.5.3
rjieba==0.1.11
rouge-score==0.1.2
rpds-py==0.10.3
rsa==4.9
ruff==0.0.259
sacrebleu==1.5.1
sacremoses==0.0.53
safetensors==0.3.2
scikit-learn==1.3.1
scipy==1.11.2
segments==2.2.1
sentencepiece==0.1.99
sigopt==8.8.2
six==1.16.0
smmap==5.0.1
sortedcontainers==2.4.0
soundfile==0.12.1
soupsieve==2.5
soxr==0.3.6
SQLAlchemy==2.0.21
SudachiDict-core==20230711
SudachiPy==0.6.7
sympy==1.12
tabulate==0.9.0
tenacity==8.2.3
tensorboard==2.13.0
tensorboard-data-server==0.7.1
tensorboardX==2.6.2.2
tensorflow==2.13.0
tensorflow-estimator==2.13.0
tensorflow-hub==0.14.0
tensorflow-io-gcs-filesystem==0.34.0
tensorflow-text==2.13.0
tensorstore==0.1.44
termcolor==2.3.0
text-unidecode==1.3
tf2onnx==1.15.1
threadpoolctl==3.2.0
timeout-decorator==0.5.0
timm==0.9.7
tokenizers==0.14.0
tomli==2.0.1
toolz==0.12.0
torch==2.0.1
torchaudio==2.0.2
torchvision==0.15.2
tqdm==4.66.1
traitlets==5.10.0
-e git+https://github.com/sbucaille/transformers.git@77f920f23b29ddf922beedace8ee3cf45a6ead95#egg=transformers
triton==2.0.0
typing_extensions==4.5.0
tzdata==2023.3
tzlocal==5.0.1
unidic==1.1.0
unidic-lite==1.0.8
uritemplate==4.1.1
urllib3==1.26.16
wasabi==0.10.1
Werkzeug==2.2.3
wrapt==1.15.0
xxhash==3.3.0
yarl==1.9.2
zipp==3.17.0
```


Hi, with your branch or the main branch, I am not able to reproduce the issue. 

What happen if you run `python3 utils/check_docstrings.py --fix_and_overwrite` instead of `make repo-consistency`. I would guess you still get the `TFRegNet` things?

If you are using the new created python environment, make sure the bash command where you use `python` is the one pointed to the new path of the python binary.

Otherwise, probably it is python version cause the difference - and I have to check it.

hi @ydshieh ,
When I run `python3 utils/check_docstrings.py --fix_and_overwrite` I don't get any error, same if I run `make fix-copies` but `make repo-consistency` always returns this TFRegNet error. What is weird is that the CI tests didn't catch the error... I can't wrap my head around this :exploding_head: 

Anyway, @rafaelpadilla, I guess all the comments were covered by my last commits, let me know if there is anything else I should do !

Yeah, I make a new python env (3.10) install everything from scratch and still can't get the error reproduced. Let's move on ..

Hi @rafaelpadilla , apart from the only conversation I have not resolved in `src/transformers/models/superpoint/modeling_superpoint.py` that should be good. I realize there was a lot of leftovers from my first files generation using transformers-cli which was originally for SuperGlue, sorry for that

> Hi @rafaelpadilla , apart from the only conversation I have not resolved in `src/transformers/models/superpoint/modeling_superpoint.py` that should be good. I realize there was a lot of leftovers from my first files generation using transformers-cli which was originally for SuperGlue, sorry for that

Hey @sbucaille, 
No worries. I think it is almost ready for a final review of one of out core maintainers.
Please, tag me for a last check when it's ready for review. :) 

Hi @rafaelpadilla, I'm sorry if I'm missing something or if my previous message was not clear but I actually need an answer to the [remaining review conversation](https://github.com/huggingface/transformers/pull/25786/files/d1aa6160c20bb9af32733fc30529e92f6e6cd179..5fed639d353201d7eba3f9baf103502e201a45f1#r1359506846) so that other reviewers may proceed, I think

Hi @rafaelpadilla, @amyeroberts, @ArthurZucker,
Thanks for the reviews, just to let you know that I'll be off for a couple of weeks due to RL stuff.
So be assured that I'll address the suggested changes at a more appropriate time, in case the PR gets considered as stale from the bot. Thanks again !

@sbucaille Thanks for letting us know! We'll keep the PR alive :) 

Hello @rafaelpadilla , @amyeroberts ,
I'm finally back and settled to continue working on this PR. I have dealt with minor changes you suggested and added my opinion on others (like [here](https://github.com/huggingface/transformers/pull/25786/files#r1371791244)). I'll deal with the bigger changes later this week and let you know :)
Thanks again for keeping this PR alive !

Sure! @amyeroberts is off this week, feel free to ping me if you need any help! 

@sbucaille Great - glad you're back and PR is active! I'm back too - feel free to ping if you have any Qs about the larger changes. 

Hi @amyeroberts, I added answers to some of the comments made in your review and fixes to others.
Let me know what you think.
Also @ArthurZucker thanks for the heads up, and sorry, I forgot to mention you in my last comment ! 

Hey everyone,
Sorry, but I didn't get any notifications of the latest reviews added to the PR, hence my response time.
I have worked mostly on implementation issues and will fix all documentation related issues at the very end. Most changes are addressed afaik.
Regarding the big diff on `docs/source/en/index.md`, I can't really explain what caused this, I suspect it comes from the merge I made [here](https://github.com/huggingface/transformers/pull/25786/commits/ea9f9ed8cdd0c6d33cca97e955731279a724e538) and I don't really know how to revert this thing.
Also, what's up with the tests ? Not sure to understand the errors, I made sure that my tests passed on my pc :thinking: 

> Regarding the big diff on docs/source/en/index.md, I can't really explain what caused this, I suspect it comes from the merge I made [here](https://github.com/huggingface/transformers/pull/25786/commits/ea9f9ed8cdd0c6d33cca97e955731279a724e538) and I don't really know how to revert this thing.

The simplest way to undo this to checkout the file from the `main` branch on `upstream` and commit the diff (which reverts the changes in this PR):
```
# Checkout the file from the main branch
git checkout upstream/main docs/source/en/index.md
git add docs/source/en/index.md
```



> Also, what's up with the tests ? Not sure to understand the errors, I made sure that my tests passed on my pc 🤔

There were a few library recent library releases e.g. pytest which caused issues in our CI runs, partly because of compatibility with our code, and partly compatibility with other libraries we require. Most of the errors I see are related to this. This should be resolved on `main` - rebasing to include the most recent commits should resolve this :) 

Hi,
Alright, I can't wrap my head around this git problem, I feel like I ended up ruining my branch with the manipulations I tried to do.
What do you think should I do ? I'm about to erase completely my branch and create a fresh one from the `main` branch, from where I could put back all the work I've done for these past months...


@sbucaille 

Sorry about this, but I am afraid there is nothing we can help on this situation (at least not from me).
One tip (for future PRs): don't use `git merge`, use `git rebase` to keep the commits linear. It will avoid such undesired situation.

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](https://github.com/huggingface/transformers/blob/main/CONTRIBUTING.md) are likely to be ignored.