[WIP] QA Loss refactoring
Author: stas00
Pull request: yes

This is a refactoring experiment as suggested at https://github.com/huggingface/transformers/issues/6204

10 models and 1 template have been refactored - I will check for more if this looks promising (the doc and new function's signature are incomplete). Let me know whether to continue or not.

the refactoring was done with:

```
perl -0777 -pi -e '
$in = <<END;
logits = self.qa_outputs(sequence_output)
        start_logits, end_logits = logits.split(1, dim=-1)
        start_logits = start_logits.squeeze(-1)
        end_logits = end_logits.squeeze(-1)

        total_loss = None
        if start_positions is not None and end_positions is not None:
            # If we are on multi-GPU, split add a dimension
            if len(start_positions.size()) > 1:
                start_positions = start_positions.squeeze(-1)
            if len(end_positions.size()) > 1:
                end_positions = end_positions.squeeze(-1)
            # sometimes the start/end positions are outside our model inputs, we ignore these terms
            ignored_index = start_logits.size(1)
            start_positions.clamp_(0, ignored_index)
            end_positions.clamp_(0, ignored_index)

            loss_fct = CrossEntropyLoss(ignore_index=ignored_index)
            start_loss = loss_fct(start_logits, start_positions)
            end_loss = loss_fct(end_logits, end_positions)
            total_loss = (start_loss + end_loss) / 2
END
s/\Q$in\E/start_logits, end_logits, total_loss = self.calc_qa_loss(sequence_output, start_positions, end_positions)\n/msg
' \
./templates/adding_a_new_model/modeling_*    ./src/transformers/modeling_*

```

@sshleifer, I'm not sure how you're going to judge the coverage change as the coverage data is unreliable at the moment: https://github.com/huggingface/transformers/issues/6317

Comments:
# [Codecov](https://codecov.io/gh/huggingface/transformers/pull/6430?src=pr&el=h1) Report
> Merging [#6430](https://codecov.io/gh/huggingface/transformers/pull/6430?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/transformers/commit/4ffea5ce2f4d154a3696b8fe2fb116fa09235700&el=desc) will **increase** coverage by `0.17%`.
> The diff coverage is `93.10%`.

[![Impacted file tree graph](https://codecov.io/gh/huggingface/transformers/pull/6430/graphs/tree.svg?width=650&height=150&src=pr&token=9qOlN6Hb1c)](https://codecov.io/gh/huggingface/transformers/pull/6430?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #6430      +/-   ##
==========================================
+ Coverage   79.89%   80.06%   +0.17%     
==========================================
  Files         153      153              
  Lines       27902    27761     -141     
==========================================
- Hits        22291    22228      -63     
+ Misses       5611     5533      -78     
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/6430?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/modeling\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/6430/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ191dGlscy5weQ==) | `87.42% <89.47%> (+0.07%)` | :arrow_up: |
| [src/transformers/modeling\_albert.py](https://codecov.io/gh/huggingface/transformers/pull/6430/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19hbGJlcnQucHk=) | `81.85% <100.00%> (-0.19%)` | :arrow_down: |
| [src/transformers/modeling\_bart.py](https://codecov.io/gh/huggingface/transformers/pull/6430/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19iYXJ0LnB5) | `95.99% <100.00%> (+0.23%)` | :arrow_up: |
| [src/transformers/modeling\_bert.py](https://codecov.io/gh/huggingface/transformers/pull/6430/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19iZXJ0LnB5) | `88.44% <100.00%> (+0.02%)` | :arrow_up: |
| [src/transformers/modeling\_electra.py](https://codecov.io/gh/huggingface/transformers/pull/6430/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19lbGVjdHJhLnB5) | `81.87% <100.00%> (-0.26%)` | :arrow_down: |
| [src/transformers/modeling\_longformer.py](https://codecov.io/gh/huggingface/transformers/pull/6430/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19sb25nZm9ybWVyLnB5) | `90.96% <100.00%> (+0.10%)` | :arrow_up: |
| [src/transformers/modeling\_mobilebert.py](https://codecov.io/gh/huggingface/transformers/pull/6430/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19tb2JpbGViZXJ0LnB5) | `89.50% <100.00%> (+0.04%)` | :arrow_up: |
| [src/transformers/modeling\_reformer.py](https://codecov.io/gh/huggingface/transformers/pull/6430/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19yZWZvcm1lci5weQ==) | `95.82% <100.00%> (+0.13%)` | :arrow_up: |
| [src/transformers/modeling\_roberta.py](https://codecov.io/gh/huggingface/transformers/pull/6430/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19yb2JlcnRhLnB5) | `96.38% <100.00%> (+0.59%)` | :arrow_up: |
| [src/transformers/modeling\_xlm.py](https://codecov.io/gh/huggingface/transformers/pull/6430/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ194bG0ucHk=) | `91.15% <100.00%> (+0.12%)` | :arrow_up: |
| ... and [13 more](https://codecov.io/gh/huggingface/transformers/pull/6430/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/transformers/pull/6430?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/huggingface/transformers/pull/6430?src=pr&el=footer). Last update [4ffea5c...4e08790](https://codecov.io/gh/huggingface/transformers/pull/6430?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


I am unsure about this as it goes against the "all in one files" policy transformers has for the models and has been highlighted as one of the main reason people like transformers in a recent survey.
Yes the alternative is duplicate code in several files, yes this is officially considered as bad computer science practice and yes it is harder for us to maintain, but the point is to have everything available in one file a researcher can easily tweak.

A similar attempt in #4944 has been ignored for the same reason.

Tagging @thomwolf and @julien-c for their thoughts.

I'd hold off then with completing this until if and when you give a green light to do so.

My idea of killing both rabbits with one shot would be writing an easy to maintain refactored code and have a tool that will unfold it for those who want it unfolded (and control the levels of how deep the unfolding goes). Does such a tool exist in python land?

> an easy to maintain refactored code and have a tool that will unfold it for those who want it unfolded (and control the levels of how deep the unfolding goes). Does such a tool exist in python land?

We explored such tools with @aaugustin a few months ago and the conclusion then was to try and build a lightweight, home-built system for this.

We could add a simple script that copies the code from somewhere into the modeling files if necessary and another to check the consistency. The first could be called during `make style` and the second during `make quality`. I was thinking of doing something similar for the `TrainingArguments` and the examples this week (adding a tweakable training arguments file for each example using Trainer), so let's see how it goes for those and then continue with model refactoring the same way?

(probably the same script with a different flag, like `black`, but yes, I like this idea)

this proved to be a failed experiment, closing this down.