Ship the data files inside the package - #37
Merged
Merged
Conversation
…nds them Reported from Colab: "No PID gains file found at /usr/local/lib/python3.13/data/pid_gains.json. Running gradient-based tuning". Two copies of the same bug. `experts/pid.py` computed `parents[3] / "data"` and `provenance.py` computed `_ROOT.parent.parent / "data"`, both of which are the repository root from a source checkout and the interpreter's lib directory from site-packages. The wheel shipped `packages = ["src/target_gym"]` and no data at all, so every pip user silently re-ran gradient tuning on their first PID and got controllers that need not match the published baselines. That undermines the central claim of the project, which is that the recorded numbers are reproducible. `data/` now lives at `src/target_gym/data/`, resolves relative to the package, and ships in the wheel (verified: all four JSON files present). A clean install into a fresh venv builds a PID in 1.8 s with no tuning message, against minutes before. `scripts/tune_pid.py` was writing to a path the loader no longer read, which would have been worse than the original bug: tuning would have appeared to succeed and changed nothing. All twenty baselines re-recorded, since experts/pid.py is a _SHARED_SOURCE. Every number came back identical to the decimal, as expected for a change that moves a file rather than a computation. Twenty-five prose references to the old `data/` paths updated across the docs, tests and scripts. CHANGELOG and roadmap entries describing past states are left as they were. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012WdRJEyGAZwKDD1EFwwH3U
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reported from Colab: "No PID gains file found at
/usr/local/lib/python3.13/data/pid_gains.json. Running gradient-based tuning".
Two copies of the same bug.
experts/pid.pycomputedparents[3] / "data"andprovenance.pycomputed_ROOT.parent.parent / "data", both of which are the repository root from a source checkout and the interpreter's lib directory from site-packages. The wheel shippedpackages = ["src/target_gym"]and no data at all, so every pip user silently re-ran gradient tuning on their first PID and got controllers that need not match the published baselines. That undermines the central claim of the project, which is that the recorded numbers are reproducible.data/now lives atsrc/target_gym/data/, resolves relative to the package, and ships in the wheel (verified: all four JSON files present). A clean install into a fresh venv builds a PID in 1.8 s with no tuning message, against minutes before.scripts/tune_pid.pywas writing to a path the loader no longer read, which would have been worse than the original bug: tuning would have appeared to succeed and changed nothing.All twenty baselines re-recorded, since experts/pid.py is a _SHARED_SOURCE. Every number came back identical to the decimal, as expected for a change that moves a file rather than a computation.
Twenty-five prose references to the old
data/paths updated across the docs, tests and scripts. CHANGELOG and roadmap entries describing past states are left as they were.