Fix unique memory address when doing group-offloading with disk #11767
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
| self.safetensors_file_path = os.path.join(self.offload_to_disk_path, f"group_{id(self)}.safetensors") | ||
| self._group_id = _group_id | ||
| short_hash = _compute_group_hash(self._group_id) | ||
| self.safetensors_file_path = os.path.join(self.offload_to_disk_path, f"group_{short_hash}.safetensors") |
There was a problem hiding this comment.
The passed in group_id argument should be unique no? I don't think we need to compute a hash.
There was a problem hiding this comment.
I don't see any edge cases, either. But having a hash is a bit more future-proof to me.
| raise ValueError("Group offloading is not enabled for the provided module.") | ||
|
|
||
|
|
||
| def _compute_group_hash(group_id): |
There was a problem hiding this comment.
Don't think we need to hash the group id strings, they should be unique already.
There was a problem hiding this comment.
| record_stream=False, | ||
| low_cpu_mem_usage=low_cpu_mem_usage, | ||
| onload_self=True, | ||
| _group_id=_GROUP_ID_LAZY_LEAF, |
There was a problem hiding this comment.
Don't think this is needed. Lazy hook doesn't do anything with it.
There was a problem hiding this comment.
You mean to exclude group_id here? In that case, it would default id(self) I think.
|
@DN6 a gentle ping |
|
@DN6 thanks for your help on this PR :) |
3 participants
What does this PR do?
#11760 (comment)