🔗 Helpful links

• 🧪 See artifacts and rendered test results at hud.pytorch.org/pr/62293
• 📄 Preview docs built from this PR

💊 CI failures summary and remediations

As of commit bb92cc1 (more details on the Dr. CI page):

• 1/1 failures introduced in this PR

1 failure not recognized by patterns:

Job	Step	Action
linux-xenial-py3.6-gcc5.4 / calculate-docker-image	Checkout PyTorch	🔁 rerun

As discussed with @albanD offline, since we cannot test with gradcheck with the parametrized spectral norm module as-is, we try to test gradcheck again with no_grad removed. See #63593. The CI should reveal whether it works with DP or not.

This was more complicated that initially thought though because there are many side effects of removing the no_grad. Its very possible we're testing completely different.

Anyway, the things changed in linked PR are:

• (repeat changes from this PR)
• remove @nograd
• no longer modify u and v in-place via out arg (because we are in grad mode now), instead we do it out of place then copy_ after so that DP would still work (though I can't test that locally). We need an extra clone before F.normalize(...) because it saves u, v for backward now. Also do the same for weight_mat
• change clone memory_format=torch.contiguous_format -> perserve_format (TODO: why does this break gradcheck??)
• In test.nn F.normalize does not support undefined gradients so we need check_undefined=False now.
• Adds retain_graph to .backward and .grad calls because u and v begin to require grad after the first forward and connects to the graph from the previous iteration

Adds retain_graph to .backward and .grad calls because u and v begin to require grad after the first forward and connects to the graph from the previous iteration

A wrapper function should be used to make sure we don't change the "original" u and v in any way I think.

In test.nn F.normalize does not support undefined gradients so we need check_undefined=False now.

That is surprising... especially ops with a single output should work out of the box.

A wrapper function should be used to make sure we don't change the "original" u and v in any way I think.

This fixed everything. The check_undefined_grad issue went away too with the new wrapper. So yeah it's not F.normalize, I should've double checked the first time.

From the linked PR, the test is failing for DP - Not sure if DP is possible when power iteration is done in grad mode because self._u needs to be modified in-place, but self._u can be a view apparently. The grad_fn is BroadcastBackward (which I could not grep) seems to return multiple views as well.

https://app.circleci.com/pipelines/github/pytorch/pytorch/366855/workflows/b0ec5ba5-b366-43d0-8846-5cf6d675e456/jobs/15480216

@soulitzer has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@soulitzer has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Codecov Report

Merging #62293 (bb92cc1) into master (94d6215) will increase coverage by 0.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #62293      +/-   ##
==========================================
+ Coverage   67.05%   67.13%   +0.08%     
==========================================
  Files         691      691              
  Lines       90496    90496              
==========================================
+ Hits        60679    60752      +73     
+ Misses      29817    29744      -73     

@soulitzer merged this pull request in 5be17ec.