huggingface / huggingface/diffusers

Confusion about `FrozenDict` in `configuration_utils.py`

Open
#9,503 3 comments 0 reactions 0 assignees View on GitHub
stale
Dominant language
Python
Stars
34.5k
Forks
7.3k
Avg merge
3d 3h
Merged PRs (30d)
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?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.