Skip to content

fix: keep the config record when a run is resumed with a different one - #104

Open
Hendrik-code wants to merge 1 commit into
mainfrom
hm/fix-trainer-provenance
Open

Hendrik-code wants to merge 1 commit into
mainfrom
hm/fix-trainer-provenance

Conversation

@Hendrik-code

Copy link
Copy Markdown
Collaborator

The defect

shutil.copy(json_path, os.path.join(self.output_folder, "transform_params_used_for_training.json"))

Unconditional. Resume a run with a different SMAUGLAB_PARAMS_JSON and the record is replaced — the file then describes a config that only the later epochs saw, while still claiming to describe the run.

That file is the only provenance a finished run carries. migration/README.md shows how much weight it bears: the sole surviving copy of one paper config is the transform_params_gpu_used_for_training.json frozen inside its run folders.

The fix

_record_config:

  • no record yet → write it, as before;
  • identical config → no-op (not even a rewrite);
  • different config → keep the original, write the new one beside it as transform_params_used_for_training_1.json (then _2, …), and warn naming both.

Nothing is lost and nothing is silently rewritten.

Tests

unit_tests/test_trainer_provenance.py: first run writes the record; resuming with the same config warns nothing and creates nothing; resuming with a different one leaves the original byte-identical, writes the numbered copy and names it in the warning; a third config does not clobber the second.

Full suite unchanged against baseline; ruff check . clean.

🤖 Generated with Claude Code

The trainer copied `SMAUGLAB_PARAMS_JSON` to
`transform_params_used_for_training.json` unconditionally, so resuming a run
with a different config replaced the record: the file then described a config
only the later epochs saw, while still claiming to describe the run. That file
is the only provenance a finished run carries.

An identical config is still a no-op. A differing one leaves the original in
place, writes itself beside it under a numbered name, and warns naming both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

1 participant