[Feature] Support losses for DDPM-based models #142
Conversation
| rescale_mode (str, optional): Mode of the loss rescale method. | ||
| Defaults to `''`. | ||
| rescale_cfg (dict, optional): Config of the loss rescale method. | ||
| log_cfgs (list[dict] | dict | optional): Configs to collect logs. |
There was a problem hiding this comment.
In fact, we do not prefer to widely use config-dict as the argument for these reasons:
- Not straightforward: Like this example, users cannot know which information should be defined in this
log_cfgsby reading the docs. Besides, the config-dict is always related to another function, which cannot be easily found. - Too complex: There are always some rules in designing the
config-dict. - Not Safe:
Dictdata is mutable and not safe.
Just a suggestion. Next time for designing new modules, try to organize the arguments and offer a clear and straightforward API.
There was a problem hiding this comment.
Each log_cfg may contain three keywords: type, prefix and reduction. To avoid using config-dict as input, we must set log_prefix and log_reduction as inputs in the initialize function.
There was a problem hiding this comment.
If so, you may specify more here and give clear and straightforward guidance.
There was a problem hiding this comment.
More doc about the design of log collection functions is added.
| assert hasattr(self, log_collect_fn) | ||
| log_collect_fn = getattr(self, log_collect_fn) | ||
|
|
||
| log_cfg_.setdefault('prefix_name', 'loss') |
There was a problem hiding this comment.
I'm wondering whether users will change this argument. It seems that users don't have any reason to change the prefix_name. Is it right? If it is right, we can directly remove the redundant codes. If not, you may specify how to use the prefix.
* support all losses needed for DDPM * fix typos * add copyright * solve divide error in quartile log collection for pt1.5 * add more docstrings * revise known problems * cvt default value of rescale_mode to None * revise known problems * fix unit test * revise docstring for ddpm losses
We support two critical features in
DDPMLoss:log_varsby configs.