Skip to content

Conversation

@8W9aG
Copy link
Contributor

@8W9aG 8W9aG commented May 16, 2025

  • Calls train when a class is defined in the train key in cog.yaml.

This is a breaking change, since currently it calls predict but uses the type definition of the train method.

* If the user makes a training request call the
train function on the predictor class
@8W9aG 8W9aG force-pushed the sackfield/call-train-function branch from 48603c9 to 3ce6e50 Compare May 16, 2025 20:55
Copy link
Contributor

@daanelson daanelson left a comment

Choose a reason for hiding this comment

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

broadly looks good; my general worry here is that this might have unexpected consequences for our existing high volume trainers (e.g. a trainer that has train but no setup).

and to be clear, this is a worry driven by paranoia as opposed to anything I see here. 😄 do we have a test somewhere that would catch an issue with that kind of trainer?

@8W9aG
Copy link
Contributor Author

8W9aG commented May 19, 2025

broadly looks good; my general worry here is that this might have unexpected consequences for our existing high volume trainers (e.g. a trainer that has train but no setup).

and to be clear, this is a worry driven by paranoia as opposed to anything I see here. 😄 do we have a test somewhere that would catch an issue with that kind of trainer?

Luckily setup is always optional so I bet that this won't matter so much

@8W9aG 8W9aG enabled auto-merge (squash) May 20, 2025 17:06
@8W9aG 8W9aG merged commit f844888 into main May 20, 2025
21 checks passed
@8W9aG 8W9aG deleted the sackfield/call-train-function branch May 20, 2025 17:18
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