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

Sample weights #120

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# spotlight results files
*_results.txt

*~
*#*

Expand Down
120 changes: 113 additions & 7 deletions spotlight/factorization/implicit.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,6 @@ def fit(self, interactions, verbose=False):
verbose: bool
Output additional information about current epoch and loss.
"""

user_ids = interactions.user_ids.astype(np.int64)
item_ids = interactions.item_ids.astype(np.int64)

Expand All @@ -220,11 +219,114 @@ def fit(self, interactions, verbose=False):

epoch_loss = 0.0

for (minibatch_num,
(batch_user,
batch_item)) in enumerate(minibatch(user_ids_tensor,
item_ids_tensor,
batch_size=self._batch_size)):
for (
minibatch_num,
(
batch_user,
batch_item,
)
) in enumerate(
minibatch(
user_ids_tensor,
item_ids_tensor,
batch_size=self._batch_size
)
):

positive_prediction = self._net(batch_user, batch_item)

if self._loss == 'adaptive_hinge':
negative_prediction = self._get_multiple_negative_predictions(
batch_user, n=self._num_negative_samples)
else:
negative_prediction = self._get_negative_prediction(batch_user)

self._optimizer.zero_grad()

loss = self._loss_func(
positive_prediction,
negative_prediction,
)
epoch_loss += loss.item()

loss.backward()
self._optimizer.step()

epoch_loss /= minibatch_num + 1

if verbose:
print('Epoch {}: loss {}'.format(epoch_num, epoch_loss))

if np.isnan(epoch_loss) or epoch_loss == 0.0:
raise ValueError('Degenerate epoch loss: {}'
.format(epoch_loss))

def fit_weighted(self, interactions, verbose=False):
Copy link
Owner

Choose a reason for hiding this comment

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

I think we should keep a single fit method. If weights are present, we use them; if not, we don't.

This could probably be accomplished by changing the minibatch function to just infinitely yield None for those arguments that are None rather than tensors. This way, it always yields None for batch_sample_weights which ties in nicely with how the loss functions are changed.

Copy link
Contributor Author

@nikisix nikisix Jul 18, 2018

Choose a reason for hiding this comment

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

Definitely tried to keep it to a single fit function at first. But I want to avoid consuming ram for an unnecessary tensor of Nones.
Would this lead to a tensor of Nones (not None) being passed to the loss functions, rather than a single None? (unless the tensor is not created by yielding Nones?)
Would also have to augment torch_utils.shuffle and add a few if all(weights==None) checks in (may slow it down a bit).

What if we just checked if weights are specified at the beginning of fit, and if they are just have fit call _fit_weighted? That way we can maintain the api of having the user always call fit, but still keep the code relatively simple.

Sequence weight question for you: I was thinking about the sequence interactions yesterday, and my guess is to make a parallel sequence tensor full of sample weights, of the same dimensions as the sequence tensor of item-ids. Does this sound right?

Copy link
Owner

Choose a reason for hiding this comment

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

No, we won't need a tensor of nones. I'll add a comment with a prototype of what I meant.

"""
Fit the model using sample weights.

When called repeatedly, model fitting will resume from
the point at which training stopped in the previous fit
call.

Parameters
----------

interactions: :class:`spotlight.interactions.Interactions`
The input dataset.

verbose: bool
Output additional information about current epoch and loss.
"""
if interactions.weights is None:
raise ValueError('''Sample weights must be specified in the
interactions object. If you don't have sample weights,
use the fit method instead''')

user_ids = interactions.user_ids.astype(np.int64)
item_ids = interactions.item_ids.astype(np.int64)
sample_weights = interactions.weights.astype(np.float32)

if not self._initialized:
self._initialize(interactions)

self._check_input(user_ids, item_ids)

for epoch_num in range(self._n_iter):

users, items, sample_weights = shuffle(
user_ids,
item_ids,
sample_weights,
random_state=self._random_state
)

user_ids_tensor = gpu(torch.from_numpy(users),
self._use_cuda)
item_ids_tensor = gpu(torch.from_numpy(items),
self._use_cuda)
sample_weights_tensor = gpu(
torch.from_numpy(sample_weights),
self._use_cuda
)

epoch_loss = 0.0

for (
minibatch_num,
(
batch_user,
batch_item,
batch_sample_weights
)
) in enumerate(
minibatch(
user_ids_tensor,
item_ids_tensor,
sample_weights_tensor,
batch_size=self._batch_size
)
):

positive_prediction = self._net(batch_user, batch_item)

Expand All @@ -236,7 +338,11 @@ def fit(self, interactions, verbose=False):

self._optimizer.zero_grad()

loss = self._loss_func(positive_prediction, negative_prediction)
loss = self._loss_func(
positive_prediction,
negative_prediction,
sample_weights=batch_sample_weights
)
epoch_loss += loss.item()

loss.backward()
Expand Down
Loading