Behaviour of `batch_eval_metrics` determines the `include_for_metrics` behaviour
Author: prabhuteja12
Pull request: no
Labels: name: bug

### System Info

Hello! 

In the [`evaluation_loop`](https://github.com/huggingface/transformers/blob/v4.51.1/src/transformers/trainer.py#L4442) method, there is an interplay between the `batch_eval_metrics` and `include_for_metrics` arguments. When `include_for_metrics` is set to `inputs`, what is sent to `compute_metrics` is the `all_inputs` object which only contains the `input_ids` ) whereas when `batch_eval_metrics` is set to `True`, `compute_metrics` is called with `inputs` which includes the `input_ids` and also any other inputs that are passed to the model. 

This behaviour is inconsistent. Can you please look into this? 

 @zach-huggingface  @SunMarc 

### Who can help?

_No response_

### Information

- [ ] The official example scripts
- [x] My own modified scripts

### Tasks

- [ ] An officially supported task in the `examples` folder (such as GLUE/SQuAD, ...)
- [ ] My own task or dataset (give details below)

### Reproduction

Any trainer example with both 
```python
include_for_metrics = ["inputs"]
batch_eval_metrics = True
```
should cause this. 

### Expected behavior

Ideally, `batch_eval_metrics` shouldn't dictate how the `compute_metrics` is called. Can this inconsistency be fixed? 

Comments:
Thanks for the report ! Can you share a minimal reproducer ? This is a bit strange since we are passing inputs_host which comes from inputs_decode and we should only select `input_ids`

```python
            main_input_name = getattr(self.model, "main_input_name", "input_ids")
            inputs_decode = (
                self._prepare_input(inputs[main_input_name]) if "inputs" in args.include_for_metrics else None
            )
```


```python
            if self.args.batch_eval_metrics:
                if self.compute_metrics is not None and preds_host is not None and labels_host is not None:
                    is_last_step = self.accelerator.gradient_state.end_of_dataloader
                    batch_kwargs = {}
                    batch_kwargs["losses"] = losses_host if "loss" in args.include_for_metrics else None
                    batch_kwargs["inputs"] = inputs_host if "inputs" in args.include_for_metrics else None
```

In the [L4351](https://github.com/huggingface/transformers/blob/v4.51.1/src/transformers/trainer.py#L4351) , the `inputs_decode` variable is the one that is attached to the `all_inputs` in L4365. This only has one column `main_input_name` which includes just the `input_ids` attributed from the input. 

On the other hand, in L4388 the code is 
`batch_kwargs["inputs"] = inputs if "inputs" in args.include_for_metrics else None` when `self.args.batch_eval_metrics` is set. This inputs is __not__ the same as the `inputs_decode` above. For a MWE, I don't have one at hand right away, but if this explanation doesn't help, I'll try to create one for this. 


Indeed, we should pass `inputs_decode` there instead. Do you want to open a PR to fix this @prabhuteja12  ? Thanks for the explanation ! 

@SunMarc Aha! I was hoping you wouldn't say that we use `input_decode` everywhere. Is there a reason why the entire `inputs` are not passed, instead of one column? 

Hello, 

This discrepancy seems to lead to another issue when using distributed training: In [L436](https://github.com/huggingface/transformers/blob/v4.51.1/src/transformers/trainer.py#L4363), gather is done on only `inputs_decode` but not all inputs. But `inputs` is what is passed to `compute_metrics` as described previously. A simple `inputs = self.gather_function(inputs)` solves the issue locally.  What do you think about this problem? 



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.

@SunMarc any ideas for this bug? 

> @SunMarc Aha! I was hoping you wouldn't say that we use input_decode everywhere. Is there a reason why the entire inputs are not passed, instead of one column?

Normally, you should only need `input_ids` from the inputs to compute the metrics. Do you feel like you need something else ? 
If so, we can add it in `include_for_metrics`. As for the disprepancies, it was something we overlooked. We shouldn't have 

`batch_kwargs["inputs"] = inputs if "inputs" in args.include_for_metrics else None when self.args.batch_eval_metrics is set.`

 If you agree with this, would you like to submit a PR ? Thanks again for your insights. 

> If so, we can add it in `include_for_metrics `

`include_for_metrics` can only handle `loss` and `inputs` to my knowledge. It doesn't allow for arbitrary attributes. 

> We shouldn't have

I was hoping for we **should** have. I don't see the trouble in giving metrics anything we want. Can you explain this?


> I was hoping for we should have. I don't see the trouble in giving metrics anything we want. Can you explain this?

Well, inputs needs to be padded + gathered if using multiple gpus. If we start accepting everything that the user put in dataloader, it will create an overhead for nothing + it might not work with the inputs are not tensors. 

> I was hoping for we should have. I don't see the trouble in giving metrics anything we want. Can you explain this?

It's been there for a long time and no one complained about it. I don't mind adding this feature if you can share with us a valable reason for. I'm not sure what you need more for computing the metrics when you have the predictions, the labels and optionally the loss and the input_ids. 

> include_for_metrics can only handle loss and inputs to my knowledge. It doesn't allow for arbitrary attributes.

Sure but we can change that if needed 

Usecase: My metrics used inputs, predictions, and metadata about inputs (grouping) such that the metric was compute within those groups and aggregated appropriately. The groups are not used by the model and thus are not part of the inputs, but are needed only for the metric computation. 



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.

Hi, I have similar issue where I need to include additional information for metrics.

Say I am doing a classification task, but I want to weight each example depending on severity.

I think a plausible solution would be allow an extra column as 'example_id' when gathering, which denotes examples and let the user handle what happens depending on that. 

Hi, 

i have a similar use case where i train on multiple datasets, and i need a way to differentiate between them. I'm currently using this batch_eval_metrics as a workaround, wanted to check in on this issue.

Thanks.

I just have a local work around for this issue. I haven't checked if the newer versions of transformers fix this (but i assume not).