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
data state restoration #64
Conversation
edaa78b
to
ec347bc
Compare
also embarks a minor debug on update_vocab being only partially removed (following #61) |
mammoth/inputters/dataset.py
Outdated
@@ -162,13 +184,13 @@ def _cast(example_dict): | |||
if self.transforms is not None else lambda x: x | |||
), | |||
stride=self.stride, | |||
offset=self.offset, | |||
offset=offset, | |||
) | |||
examples = map(_cast, examples) | |||
yield from examples | |||
|
|||
# FIXME: some RNN archs require sorting src's by length |
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.
Out of this PR concenrs... do we still support RNNs? if not, we can just delete this
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.
no, we no longer support RNNs. the move is to externalize transformer variantsvia lucidrains (see #56). Will remove the comment
mammoth/models/model_saver.py
Outdated
@@ -95,12 +96,14 @@ def save(self, step, moving_average=None): | |||
self._rm_checkpoint(todel) | |||
self.checkpoint_queue.append(chkpt_names) | |||
|
|||
def _save(self, step): | |||
def _save(self, step, save_model, data_state, device_context): | |||
"""Save a resumable checkpoint. | |||
|
|||
Args: | |||
step (int): step number | |||
model (nn.Module): torch model to save |
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.
missing: save_model (type): description
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.
yeah, i think it was renamed from model to save_model? not sure why. will fix that.
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 :)
closes #63 .
same idea as v2, didn't bother porting from it.
does not embark bucket states, although this could maybe be done by picking the line indices from all examples in the reservoir. Would come at a (potentially high) communication overhead cost.