huggingface / huggingface/diffusers

Confusion about `FrozenDict` in `configuration_utils.py`

Ouverte
#9,503 3 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
stale
Langage dominant
Python
Étoiles
34.5k
Forks
7.3k
Merge moyen
3 j 3 h
PR mergées (30 j)
91

Description

I am confused about the design of `FrozenDict` in `configuration_utils.py` and the usage of it.

### 1. Is `FrozenDict` really frozen?
From the code, `FrozenDict` sets `self.__frozen = True` during initialization. It then checks `if hasattr(self, "__frozen") and self.__frozen` in methods like `__setattr__` or `__setitem__` to raise an exception if it's supposed to be frozen. However, in Python, using double underscores (`__`) triggers name mangling, which means `hasattr(self, "__frozen")` couldn't find `__frozen` as it is `_FrozenDict__frozen` actually. This check would always return False, rendering the intended freeze ineffective – `__setattr__` and `__setitem__` can still be used, making the `FrozenDict` not truly frozen.

Is this what we expect?

### 2. If we were to modify `FrozenDict` to be truly immutable, how would we use it as `model.config`?
Typically, we register parameters needed for model initialization as a `FrozenDict` via `register_to_config`. These are often accessed during the forward method with checks like `if self.config.xxx == xxx` to determine execution paths. However, in some models, certain properties of `self.config` might need to be altered after initialization, as seen in [IP-Adapter](https://github.com/huggingface/diffusers/blob/main/src/diffusers/loaders/unet.py#L851)
```py
self.config.encoder_hid_dim_type = "ip_image_proj"
```
Does this contradict the design philosophy of `FrozenDict`?

### 3. If `models.config` could be mutable, how should one go about changing it?

In the example above, should we use `__setattr__` to modify `self.config.encoder_hid_dim_type` or `__setitem__` to add an additional key-value pair to `FrozenDict` which appears more in line with typical dictionary usage, like
```diff
- self.config.encoder_hid_dim_type = "ip_image_proj"
+ self.config['encoder_hid_dim_type'] = "ip_image_proj"
```



I was just wondering if you might have a moment to clarify these points for me?

Guide de contribution

Ouvrir le guide de contribution

Piste de recherche

Start by reading configuration_utils.py, then compare FrozenDict's behavior with the self.config.encoder_hid_dim_type assignment in loaders/unet.py. Clarify whether the intended outcome is immutable configuration, supported mutation, or documentation of current behavior; the issue is done only once that scope and expected behavior are decided.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
python
Domaine
machine-learning
Type d'issue
Bug
Difficulté
5/5
Temps estimé
Plus d'une semaine
Activité
À l'abandon
Clarté
À clarifier
Accessibilité débutants
20/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.