Skip to content
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

Small Fixes #14

Closed
wants to merge 5 commits into from
Closed

Small Fixes #14

wants to merge 5 commits into from

Conversation

tcapelle
Copy link

This PR fixes the missing args for show_images and adds the scheduler to the learner object

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

def before_fit(self, learn): self.schedo = self.sched(learn.opt)
def before_fit(self, learn):
self.schedo = self.sched(learn.opt)
learn.schedo = self.schedo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this for?

Copy link
Author

@tcapelle tcapelle Jan 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To have access to the scheduler inside the Learner object. This way, I can monitor the learning rate or other scheduler params.
Iniside the MonitorCB now I can access the learn.schedo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you do that using the optimiser hyperparams instead?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, you are right =)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opt.param_groups[0]["lr"] returns the current learning rate from the optimizer object

@jph00
Copy link
Member

jph00 commented Jan 27, 2023

Thanks! Can you make this a more minimal diff without all the changes to images and other outputs, and remove the stacktrace that appears in one notebook?

@tcapelle
Copy link
Author

Ok, I did nbdev_clean --clear_all

@jph00
Copy link
Member

jph00 commented Jan 29, 2023

Unfortunately that deleted all the outputs!

@tcapelle
Copy link
Author

Ok, re run everything, but lot's of warning on M1Pro

@tcapelle
Copy link
Author

I will close this then... don't know how to resolve the conflicts

@tcapelle tcapelle closed this Jan 30, 2023
@@ -61,7 +61,7 @@ def subplots(
if figsize is None: figsize=(ncols*imsize, nrows*imsize)
fig,ax = plt.subplots(nrows, ncols, figsize=figsize, **kwargs)
if suptitle is not None: fig.suptitle(suptitle)
if nrows*ncols==1: ax = array([ax])
if nrows*ncols==1: ax = np.array([ax])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems to already be in master

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

=)

@jph00
Copy link
Member

jph00 commented Jan 30, 2023

I'll fix show_images directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants