-
Notifications
You must be signed in to change notification settings - Fork 3.5k
loop customization docs #9609
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
loop customization docs #9609
Conversation
for more information, see https://pre-commit.ci
Co-authored-by: Carlos Mocholí <[email protected]>
Co-authored-by: thomas chaton <[email protected]>
for more information, see https://pre-commit.ci
ca89b80
to
dda835a
Compare
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Big improvement compared to the previous iteration, we can still improve, up to us when to do it. I have left comments inline.
A comment on the graphics: I really like the gifs, but I noticed that except for the first gif, the last frame only persists on screen for a fraction of a second, while it should stay there for a full few seconds. It's really hard to consume it right now.
# train generator, then yield | ||
fake_pred = self.discriminator(fake) | ||
gen_loss = self.criterion(fake_pred, fake_gt) | ||
yield gen_loss |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example is not easy to follow, I had to go back and forth a few times to grasp it. We should probably make an effort at making it more digestible by avoiding glossing over details, and explaining (in words or visually) how the generator will be advanced once per optimizer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Luca, I agree it's quite complex. The use case is simple but the implementation at the moment is not so easy. As we will continue to improve access to loop customization, this should become much cleaner. For now, I'm taking the example out of the docs and will create a tutorial example instead here: #9983
Plus, we will add more api docs to the loops, especially for the undocumented methods, so they become accessible through our doc pages.
Codecov Report
@@ Coverage Diff @@
## master #9609 +/- ##
=======================================
- Coverage 93% 93% -0%
=======================================
Files 180 179 -1
Lines 15090 15829 +739
=======================================
+ Hits 14019 14670 +651
- Misses 1071 1159 +88 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
Just a note, better to replace all references to classes and functions with cross references (:class:~pytorch_lightning.core.optimizer.LightningOptimizer
etc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Carlos Mocholí <[email protected]> Co-authored-by: thomas chaton <[email protected]> Co-authored-by: edenlightning <[email protected]>
val_loss = 0 | ||
for i, val_batch in enumerate(val_dataloader): | ||
x, y = val_batch | ||
y_hat = model(x) | ||
val_loss += loss_function(y_hat, y) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks a bit strange running full validation loop for each training step/batch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we want to enable strange things with loop customization :)) we wouldn't be able to do this so cleanly in pure Lightning. That's the core value haha
optimizer.step() | ||
|
||
However, some new research use cases such as meta-learning, active learning, recommendation systems, etc., require a different loop structure. | ||
For example here is a simple loop that guides the weight updates with a loss from a special validation split: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we refer to some literature? this kind of weight update is not very common...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the closest I could think of is bilevel learning: https://arxiv.org/pdf/1809.01465.pdf
What I sketched here in the docs is not exactly that but I couldn't add too many details because the focus should be on the illustration of the loop structure, less on the details of how such an approach would be useful in practice.
Each loop has a series of methods that can be modified. | ||
For example with the :class:`~pytorch_lightning.loops.fit_loop.FitLoop`: | ||
|
||
.. code-block:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. code-block:: python
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pre-commit/blacken-docs was not happy. couldn't figure out the problem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but others seem to have it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, in other sections blacken-docs does not fail with a cryptic error :(
If you find the fix I will buy you a beer/coffee
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I eligible for the free beer?
It fails because your forgot the ":" in the on_advance_end
def
Also, the advance
def should take self
for consistency (but this does not make it fail).
🍺🍺🍺
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thank you very much!
#10036
Send me your home address on slack, I'll ship the beer asap 😃
|
||
.. _override default loops: | ||
|
||
Overriding the default loops |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just thinking is all loops kind if tested with in the Traner sanity check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope, only the validation loop :)
What does this PR do?
Continuation of #9557 with new structure.
TODO:
Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
Before you start reviewing make sure you have read Review guidelines. In short, see the following bullet-list:
Did you have fun?
I made sure I had fun coding 🙃