Skip to content

Conversation

@vfdev-5
Copy link
Collaborator

@vfdev-5 vfdev-5 commented Jan 20, 2021

Fixes #1521

Description:

  • bug fixes
  • state_dict/load_state_dict refactor
  • added helper Engine.debug() method
  • more tests

Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

@vfdev-5 vfdev-5 changed the title Improved max_iters handing Improved max_iters handling Apr 19, 2021

_state_dict_all_req_keys = ("epoch_length", "max_epochs")
_state_dict_one_of_opt_keys = ("iteration", "epoch")
_state_dict_all_req_keys = ("epoch_length",)
Copy link
Collaborator

Choose a reason for hiding this comment

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

But if we interrupted the engine during the first epoch, we would not have epoch_length.

_state_dict_all_req_keys = ("epoch_length", "max_epochs")
_state_dict_one_of_opt_keys = ("iteration", "epoch")
_state_dict_all_req_keys = ("epoch_length",)
_state_dict_one_of_opt_keys = (("iteration", "epoch",), ("max_epochs", "max_iters",))
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can relax the constraint that the both max_epochs and max_iters could not have values.

if self.state.epoch_length is not None:
self.state.epoch = self.state.iteration // self.state.epoch_length
elif "epoch" in state_dict:
else:
Copy link
Collaborator

Choose a reason for hiding this comment

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

According to state_dict this should never happen.

self.state.max_iters = max_iters

def _check_and_set_epoch_length(self, data: Iterable, epoch_length: Optional[int] = None) -> None:
# Can't we accept a redefinition ?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can't because the relationship between epoch and iteration becomes invalid then. By the way in the case epoch_length is changed, we could reinitialize the state (with epoch and iteration zero along with a message to the user) as well.


return time.time() - start_time

def debug(self, enabled: bool = True) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you still want to merge this method into the master?

@leej3 leej3 mentioned this pull request Apr 17, 2024
3 tasks
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.

Possible issues with max_iters when loading/saving engine's state

4 participants