diffusers icon indicating copy to clipboard operation
diffusers copied to clipboard

Confusion about `FrozenDict` in `configuration_utils.py`

Open townwish4git opened this issue 1 year ago • 2 comments

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

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

- 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?

townwish4git avatar Sep 23 '24 15:09 townwish4git

I don't know if this will contribute to the discussion meaningfully but there is a method in ConfigMixin that allows changing of FrozenDict this way:

self.register_to_config(cross_attention_dim=256)

tahahah avatar Oct 17 '24 00:10 tahahah

I don't know if this will contribute to the discussion meaningfully but there is a method in ConfigMixin that allows changing of FrozenDict this way:

self.register_to_config(cross_attention_dim=256)

Thanks a lot for resolving my confusion about 2&3. So it looks like self.config should not be modified directly, but should be updated through the provided register_to_config interface. Thanks again!

townwish4git avatar Oct 17 '24 02:10 townwish4git

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 are likely to be ignored.

github-actions[bot] avatar Nov 10 '24 15:11 github-actions[bot]