huggingface / huggingface/diffusers

Confusion about `FrozenDict` in `configuration_utils.py`

Aperta
#9,503 3 commenti 0 reazioni 0 assegnatari Vedi su GitHub
stale
Lingua principale
Python
Stelle
34.5k
Fork
7.3k
Merge medio
3g 3h
PR unite (30g)
91

Descrizione

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?

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.