feat: use bagua_module_name to identify different modules #438
Conversation
| """ | ||
| return self._bagua_module_name | ||
|
|
||
| @bagua_module_name.setter |
There was a problem hiding this comment.
Why not use .bagua_module_name directly?
There was a problem hiding this comment.
Yes, we can use bagua_module_name directly, I just think it looks clearer.
| @@ -99,24 +99,30 @@ def with_bagua( # pytype: disable=module-attr | |||
| if process_group is None: | |||
| process_group = _get_default_group() | |||
|
|
|||
| bagua_module_name = None | |||
| if hasattr(self, "bagua_ddp"): | |||
| bagua_module_name = self.bagua_ddp.bagua_module_name | |||
There was a problem hiding this comment.
In my understanding, it cannot use .with_bagua and BaguaDistributedParallel at the same time. So may not need to use separate .bagua_module_name to identify different modules.
There was a problem hiding this comment.
I want to make it possible for our users to tell us whether two module instances are the same or not, instead of simply being determined by the their ids.
In fact, this comes from a dev requirement from Lightning. A torch.nn.Module can be wrapped multiple times, but they share same model parameters.
There was a problem hiding this comment.
I want to make it possible for our users to tell us whether two module instances are the same or not, instead of simply being determined by the their ids.
Users only need to print the module.
My core point is .with_bagua is deprecated, we should not use it anymore.
pr-triage
bot
added
PR: partially-approved
and removed
PR: reviewed-changes-requested
labels
No description provided.