Skip to content

Conversation

@le1nux
Copy link
Member

@le1nux le1nux commented Feb 13, 2025

What does this PR do?

This PR simplifies warmstarts from a checkpoint by adding a warmstart entrypoint.
We pass the warmstart config and the checkpoint info file to the endpoint. The checkpoint info file contains the path information for the model and optimizer checkpoint. The checkpoint path is injected into the config via a pydantiv env resolver.

Checklist before submitting final PR

  • My PR is minimal and addresses one issue in isolation
  • I have merged the latest version of the target branch into this feature branch
  • I have reviewed my own code w.r.t. correct implementation, missing type hints, proper documentation, etc.
  • I have run a sample config for model training
  • I have checked that all tests run through (python tests/tests.py)
  • I have updated the internal changelog (CHANGELOG_DEV.md)

@le1nux le1nux requested review from flxst, fromm-m and mali-git February 13, 2025 22:44
@le1nux le1nux self-assigned this Feb 14, 2025
@le1nux le1nux added the enhancement New feature or request label Feb 14, 2025
Copy link
Member

@fromm-m fromm-m left a comment

Choose a reason for hiding this comment

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

great work!

Copy link
Member

@flxst flxst left a comment

Choose a reason for hiding this comment

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

Looks good, but I think the end2end test might not be sufficient, see my comment.

@le1nux le1nux requested review from flxst and fromm-m February 19, 2025 15:42
Copy link
Member

@flxst flxst left a comment

Choose a reason for hiding this comment

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

LGTM! Added a few comments that should perhaps be addressed, nothing serious though.

# cd to the directory of the script (absolute pathe)
cd "$(dirname "$0")"

rm -rf ../data/
Copy link
Member

@flxst flxst Feb 20, 2025

Choose a reason for hiding this comment

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

I wonder if we should move this line to the end of the shell script, right above the final "Finished warmstart example" message. This would clean up the temporary data right after the test, instead of cleaning up the data of a previous test at the beginning. (This is what is done in the case of the getting started example test as well).

@le1nux le1nux merged commit 0201531 into main Feb 20, 2025
3 checks passed
@le1nux le1nux deleted the warmstart_automation_simple branch February 20, 2025 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants