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

Change checkpoint suffix to "ckpt" #3470

Merged
merged 2 commits into from
Feb 19, 2020
Merged

Change checkpoint suffix to "ckpt" #3470

merged 2 commits into from
Feb 19, 2020

Conversation

harperj
Copy link
Contributor

@harperj harperj commented Feb 18, 2020

Tensorflow doesn't prescribe any particular file suffix for checkpoint files, but they
are commonly referred to as "ckpt" as a shorthand for "checkpoint". However ours
is somewhat confusingly "cptk". This change simply changes our checkpoint suffix
to "ckpt".

Proposed change(s)

Change the checkpoint file suffix to "ckpt".

Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)

Brought up in #3456

Types of change(s)

  • Bug fix
  • New feature
  • Code refactor
  • Breaking change
  • Documentation update
  • Other (please describe)

Minor interface change.

Checklist

  • I have added tests that prove my fix is effective or that my feature
    This should be covered by existing testing.

  • I have added updated the changelog (if applicable)

  • I have added necessary documentation (if applicable)

  • I have updated the migration guide (if applicable)

For the above, I believe this should not affect loading or saving existing training runs,
and probably is below the threshold of requiring an entry in the changelog (but open
to disagreement on this).

Other comments

Tensorflow doesn't prescribe any particular file suffix for checkpoint files, but they 
are commonly referred to as "ckpt" as a shorthand for "checkpoint".  However ours 
is somewhat confusingly "cptk".  This change simply changes our checkpoint suffix 
to "ckpt".
@harperj harperj requested a review from chriselion February 18, 2020 23:29
Copy link
Contributor

@chriselion chriselion left a comment

Choose a reason for hiding this comment

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

I think it's worth mentioning in the changelog.

@harperj
Copy link
Contributor Author

harperj commented Feb 19, 2020

Changelog updated. Thanks 🚢

@harperj harperj merged commit d6d134a into master Feb 19, 2020
@delete-merged-branch delete-merged-branch bot deleted the develop-ckpt-suffix branch February 19, 2020 01:01
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants