Skip to content

CLEAN : rm duplicate code in fit_loop. #564

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Feb 10, 2020
Merged

CLEAN : rm duplicate code in fit_loop. #564

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
74b0068
CLEAN : rm duplicate code in fit_loop.
Nov 26, 2019
c2371dd
Add doc and clean signature of single_epoch.
Dec 5, 2019
791efe7
Change name run_single_epoch
YannDubs Dec 9, 2019
1bfff85
Apply suggestions from code review (formatting)
YannDubs Jan 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 30 additions & 32 deletions skorch/net.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -725,43 +725,41 @@ def fit_loop(self, X, y=None, epochs=None, **fit_params):
'dataset_valid': dataset_valid,
}

y_train_is_ph = uses_placeholder_y(dataset_train)
y_valid_is_ph = uses_placeholder_y(dataset_valid)

for _ in range(epochs):
for epoch in range(epochs):
self.notify('on_epoch_begin', **on_epoch_kwargs)

train_batch_count = 0
for data in self.get_iterator(dataset_train, training=True):
Xi, yi = unpack_data(data)
yi_res = yi if not y_train_is_ph else None
self.notify('on_batch_begin', X=Xi, y=yi_res, training=True)
step = self.train_step(Xi, yi, **fit_params)
train_batch_count += 1
self.history.record_batch('train_loss', step['loss'].item())
self.history.record_batch('train_batch_size', get_len(Xi))
self.notify('on_batch_end', X=Xi, y=yi_res, training=True, **step)
self.history.record("train_batch_count", train_batch_count)

if dataset_valid is None:
self.notify('on_epoch_end', **on_epoch_kwargs)
continue
self._single_epoch(dataset_train, training=True, epoch=epoch, **fit_params)

valid_batch_count = 0
for data in self.get_iterator(dataset_valid, training=False):
Xi, yi = unpack_data(data)
yi_res = yi if not y_valid_is_ph else None
self.notify('on_batch_begin', X=Xi, y=yi_res, training=False)
step = self.validation_step(Xi, yi, **fit_params)
valid_batch_count += 1
self.history.record_batch('valid_loss', step['loss'].item())
self.history.record_batch('valid_batch_size', get_len(Xi))
self.notify('on_batch_end', X=Xi, y=yi_res, training=False, **step)
self.history.record("valid_batch_count", valid_batch_count)

self.notify('on_epoch_end', **on_epoch_kwargs)
if dataset_valid is not None:
self._single_epoch(dataset_valid, training=False, epoch=epoch, **fit_params)

self.notify("on_epoch_end", **on_epoch_kwargs)
return self

def _single_epoch(self, dataset, training, epoch, **fit_params):
Copy link
Member

Choose a reason for hiding this comment

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

We can update this signature to

def _single_epoch(self, dataset, training, prefix, 
                  step_fn, epoch, **fit_params):
    ...

And let the caller pass the arguments in:

 for epoch in range(epochs):
    self.notify('on_epoch_begin', **on_epoch_kwargs)
    self._single_epoch(dataset_train, training=True, prefix="train",
					   step_fn=self.train_step, epoch=epoch, **fit_params)

    if dataset_valid is not None:
        self._single_epoch(dataset_valid, training=False, prefix="valid",
					       step_fn=self.validation_step, epoch=epoch, **fit_params)
    self.notify("on_epoch_end", **on_epoch_kwargs)

"""Computes a single epoch of train or validation."""
is_placeholder_y = uses_placeholder_y(dataset)

if training:
prfx = "train"
step_fn = self.train_step
else:
prfx = "valid"
step_fn = self.validation_step

batch_count = 0
for data in self.get_iterator(dataset, training=training):
Xi, yi = unpack_data(data)
yi_res = yi if not is_placeholder_y else None
self.notify("on_batch_begin", X=Xi, y=yi_res, training=training)
step = step_fn(Xi, yi, **fit_params)
self.history.record_batch(prfx + "_loss", step["loss"].item())
self.history.record_batch(prfx + "_batch_size", get_len(Xi))
self.notify("on_batch_end", X=Xi, y=yi_res, training=training, **step)
batch_count += 1

self.history.record(prfx + "_batch_count", batch_count)

# pylint: disable=unused-argument
def partial_fit(self, X, y=None, classes=None, **fit_params):
"""Fit the module.
Expand Down