Skip to content

Conversation

@mryab
Copy link
Member

@mryab mryab commented Mar 31, 2025

The introduction of Literal in #273 broke parsing of old jobs without the learning rate scheduler specified, as Pydantic receives an empty string for the type and can't match it to any of the LR scheduler types that we have defined.

The error looks like this:

data.4.lr_scheduler.FinetuneCosineLRScheduler.lr_scheduler_type
  Input should be 'cosine' [type=literal_error, input_value='', input_type=str]
    For further information visit https://errors.pydantic.dev/2.10/v/literal_error

As a simple fix before any backend changes, this PR adds an empty scheduler type and enables it only in FinetuneResponse. together fine-tuning list works with the new fix

The PR also makes a few cleanups around the codebase related to LR scheduling and fixes a hidden bug introduced in the previous PR, where lr_scheduler_args was renamed to lr_scheduler but Pydantic silently accepted the previous name of the field

Copy link
Member Author

Choose a reason for hiding this comment

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

As you can see here, we passed lr_scheduler_args despite this field not being defined for FinetuneCosineLRScheduler

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops ty for the catch. I'm actually not sure it didn't have a validation error before...

Choose a reason for hiding this comment

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

Smart! Looks good to me if we have no other options to support old jobs

@mryab mryab force-pushed the fix-empty-scheduler branch from 70be7ec to eeafb4a Compare April 2, 2025 17:35
@mryab mryab merged commit cd154f8 into main Apr 2, 2025
9 checks passed
@mryab mryab deleted the fix-empty-scheduler branch April 2, 2025 17:39
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.

4 participants