Fix the duplication problem in _unique_state_dict #18139
Conversation
|
cc: @spandantiwari |
facebook-github-bot
added
the
oncall: jit
|
Might be worth a comment explaining why we do it this way though |
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@houseroad - LGTM. I can double check it on my side but I am sure that this will fix the issue that we were seeing. |
|
Also, since it fixes a bug - add a unittest for previously failing case as a good eng practice? |
|
Yes, will add a test @dzhulgakov |
houseroad
force-pushed
the
fix_unique_state_dict
branch
from
879826b to
a1f420b
Compare
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@dzhulgakov test case has been added. Does it look good to you? :-) |
|
@houseroad merged this pull request in 0c237f1. |
5 participants
Since parameter.data will create a new torch.Tensor each time, we get duplicate tensors when call _unique_state_dict now. Try to deduplicate it before creating new tensor.