Repository: huggingface/transformers
File: src/transformers/utils/generic.py (Python)

Code before:
                        if arg_value not in valid_strategies:
                            raise ValueError(
                                f"`Unexpected select feature strategy: {arg_value}. "
                                f"Please select from {valid_strategies}."
                            )

                    if arg_index is not None and len(args) > arg_index:
                        args = list(args)
                        args[arg_index] = arg_value
                        args = tuple(args)
                    else:
                        kwargs[arg_name] = arg_value

            return_dict = kwargs.pop("return_dict", None)
            if return_dict is None:
                return_dict = getattr(self.config, "return_dict", True)

            # Maybe temporarily overwrite config value to create the correct mask - kwarg takes precedence
            is_causal = kwargs.get("is_causal", True) and getattr(self.config, "is_causal", True)
            if not is_causal:
                is_causal_in_config = hasattr(self.config, "is_causal")
                if is_causal_in_config:
                    is_causal_original_value = self.config.is_causal
                # Set it to both config and kwargs (it's needed in both, and can come from only 1 of the sources)
                self.config.is_causal = False
                kwargs["is_causal"] = False

            all_args = kwargs.copy()
            if "kwargs" in all_args:
                for k, v in all_args["kwargs"].items():
                    all_args[k] = v

            # _can_record_outputs is None by default
            capture_flags = _CAN_RECORD_REGISTRY.get(str(self.__class__)) or {}  # there is a weak ref for executorch
            recordable_keys = {
                f"output_{k}": all_args.get(
                    f"output_{k}",
                    getattr(
                        self.config,
                        f"output_{k}",
                        all_args.get("output_attentions", getattr(self.config, "output_attentions", False)),
                    ),
                )
                for k in capture_flags
            }

            # We let cross attentions to be saved separately because some models add `cross-attn` layer
            # when certain condtions are met. Let's output cross attention if attentions are requested (for BC)
            if "output_attentions" in recordable_keys:
                recordable_keys["output_cross_attentions"] = recordable_keys["output_attentions"]


Review comment (suggestion):
If I understand correctly, then the final `is_causal` is:
|                              | kwargs["is_causal"] = True | kwargs["is_causal"] = False | kwargs["is_causal"] not defined |
|------------------------------|----------------------------|-----------------------------|---------------------------------|
| config.is_causal = True      | True                       | False                       | True                            |
| config.is_causal = False     | **False**                  | False                       | False                           |
| config.is_causal not defined | True                       | False                       | True                            |

The bold False here is an outlier: if the architecture is bidirectional in nature (i.e. `config.is_causal=False`), then the user can't override that to causal. Personally, I'm exclusively interested in the decoder -> encoder case, so this is not a problem for my use cases, but perhaps we want to allow the `kwargs` to always have priority?

Then we'd instead use something like this
```suggestion
            # Maybe temporarily overwrite config value to create the correct mask - kwarg takes precedence
            is_causal = kwargs.get("is_causal", getattr(self.config, "is_causal", True))
            is_causal_in_config = hasattr(self.config, "is_causal")
            if is_causal_in_config:
                is_causal_original_value = self.config.is_causal
            # Set it to both config and kwargs (it's needed in both, and can come from only 1 of the sources)
            self.config.is_causal = is_causal
            kwargs["is_causal"] = is_causal
```

And then also drop the `if not is_causal` a little later on:
```python
            if is_causal_in_config:
                self.config.is_causal = is_causal_original_value
            else:
                del self.config.is_causal
```


Code after:
                        if arg_value not in valid_strategies:
                            raise ValueError(
                                f"`Unexpected select feature strategy: {arg_value}. "
                                f"Please select from {valid_strategies}."
                            )

                    if arg_index is not None and len(args) > arg_index:
                        args = list(args)
                        args[arg_index] = arg_value
                        args = tuple(args)
                    else:
                        kwargs[arg_name] = arg_value

            return_dict = kwargs.pop("return_dict", None)
            if return_dict is None:
                return_dict = getattr(self.config, "return_dict", True)

            # Maybe temporarily overwrite config value to create the correct mask - kwarg takes precedence
            is_causal = kwargs.get("is_causal")
            if is_causal is None:
                is_causal = getattr(self.config, "is_causal", None)

            if is_causal is not None:
                is_causal_in_config = hasattr(self.config, "is_causal")
                if is_causal_in_config:
                    is_causal_original_value = self.config.is_causal
                # Set it to both config and kwargs (it's needed in both, and can come from only 1 of the sources)
                self.config.is_causal = is_causal
                kwargs["is_causal"] = is_causal

            all_args = kwargs.copy()
            if "kwargs" in all_args:
                for k, v in all_args["kwargs"].items():
                    all_args[k] = v

            # _can_record_outputs is None by default
            capture_flags = _CAN_RECORD_REGISTRY.get(str(self.__class__)) or {}  # there is a weak ref for executorch
            recordable_keys = {
                f"output_{k}": all_args.get(
                    f"output_{k}",
                    getattr(
                        self.config,
                        f"output_{k}",
                        all_args.get("output_attentions", getattr(self.config, "output_attentions", False)),
                    ),
                )
                for k in capture_flags
            }

            # We let cross attentions to be saved separately because some models add `cross-attn` layer
            # when certain condtions are met. Let's output cross attention if attentions are requested (for BC)