Skip to content

Conversation

@priorphil
Copy link
Contributor

Previously, if the checkpoint is loaded and re-exported, the inference config is set, but to None.

Issue

Please link the corresponding GitHub issue. If an issue does not already exist,
please open one to describe the bug or feature request before creating a pull request.

This allows us to discuss the proposal and helps avoid unnecessary work.

Motivation and Context


Public API Changes

  • No Public API changes
  • Yes, Public API changes (Details below)

How Has This Been Tested?


Checklist

  • The changes have been tested locally.
  • Documentation has been updated (if the public API or usage changes).
  • A entry has been added to CHANGELOG.md (if relevant for users).
  • The code follows the project's style guidelines.
  • I have considered the impact of these changes on the public API.

Previously, if the checkpoint is loaded and re-exported, the inference config is set, but to None.
@priorphil priorphil requested a review from oscarkey November 3, 2025 19:24
@priorphil priorphil requested a review from a team as a code owner November 3, 2025 19:24
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly addresses a potential TypeError when loading a model checkpoint where inference_config is present but set to None. The change is a good safeguard. I've suggested a slightly more concise and robust way to perform this check using dict.get() and a truthiness check, which also handles cases where inference_config might be an empty dictionary.

Copy link
Contributor

@oscarkey oscarkey left a comment

Choose a reason for hiding this comment

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

ahhh nice catch! Thank you

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@priorphil priorphil enabled auto-merge (squash) November 3, 2025 21:09
@priorphil priorphil merged commit 7fc02dd into main Nov 3, 2025
10 checks passed
oscarkey pushed a commit that referenced this pull request Nov 12, 2025
…oading. (#227)

* Record copied public PR 586

* Check that inference_config is actually set when loading. (#586)

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
(cherry picked from commit 7fc02dd)

---------

Co-authored-by: mirror-bot <mirror-bot@users.noreply.github.com>
Co-authored-by: Phil <philipp@priorlabs.ai>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
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.

3 participants