fix: record stride/padding op_params for TS-001's conv2d cases - #3
Merged
Merged
Conversation
The three UC files below set stride=2/padding=1 on the real torch.nn.Conv2d call, so the recorded result tensor was always correct -- but never passed the same value via @eXecutable's op_params, so nothing in the GGUF file's metadata recorded that a non-default parameter was used at all. Found by SKaiNET's skainet-test-groundtruth switching from guessing parameters out of the human-readable description text to reading them from op.* GGUF metadata directly (see CONTRACT.md, added here) -- doing that correctly surfaced that this data was simply missing, not that the Kotlin-side reader had a bug. Before this fix, SKaiNET's own conv2d correctly ran with default stride=1/padding=0 (since it had no other information), producing a 30x30 output where PyTorch's ground truth (stride=2 or padding=1 applied) was 15x15/32x32 -- a 100% shape/value mismatch that looked like a real numerical bug until traced back here. - UC-001.py's second function (also named strided_convolution, a near-duplicate of UC-002.py's) and UC-002.py: op_params={"stride": 2} - UC-003.py: op_params={"padding": 1} Also fixes every remaining git+https://github.com/sk-ai-net/... URL (pytorch/requirements.txt, pytorch/pyproject.toml, AGENTS.md, README.md -- including README's own self-referential clone URL) to point at the project's current SKaiNET-developers org instead of its old sk-ai-net name. requirements.txt's stale URL isn't just cosmetic: it means `pip install -r requirements.txt` pulls gradienttracer from wherever sk-ai-net/gradienttracer currently resolves to (a different, likely unmaintained fork/remnant) rather than the actively developed SKaiNET-developers/gradienttracer this project is meant to track. Adds CONTRACT.md: the GGUF tensor-naming/metadata contract (input_N, result, op.* params, general.description/name) other consumers -- SKaiNET today, potentially a future numcrux-hosted corpus -- need to agree on without reading this repo's source. Verified: rebuilt the Docker image (confirms pip resolves the fixed gradienttracer URL correctly), regenerated all 7 test suites, and confirmed via SKaiNET's skainet-test-groundtruth module that TS-001 goes from 3/6 to 6/6 passing with these op_params in place.
Merged
4 tasks done
Contributor
Author
|
Companion SKaiNET PR (the metadata-driven params change that surfaced this gap, plus the new CI workflow): SKaiNET-developers/SKaiNET#989 |
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.
Summary
Found while making SKaiNET's
skainet-test-groundtruthmodule read operationparameters from GGUF
op.*metadata instead of guessing them from the human-readabledescription text (see the companion SKaiNET PR — link added once opened).
Three
@Executablefunctions inTS-001(UC-001.py's second function,UC-002.py,UC-003.py) setstride=2/padding=1on the realtorch.nn.Conv2dcall — so therecorded
resulttensor was always numerically correct — but never passed the samevalue via
op_params, so nothing in the GGUF file recorded that a non-defaultparameter was used at all. Reading real metadata instead of guessing from text
immediately surfaced this: SKaiNET's own conv2d correctly ran with default
stride=1/padding=0 (its only information), producing a 30×30 output against PyTorch's
15×15/32×32 ground truth — a 100% shape/value mismatch that looked like a genuine
numerical bug in SKaiNET until traced back to this missing metadata.
Changes
UC-001.py,UC-002.py:op_params={"stride": 2}UC-003.py:op_params={"padding": 1}git+https://github.com/sk-ai-net/...URL (requirements.txt,pyproject.toml,AGENTS.md,README.md— including the README's ownself-referential clone URL) to
SKaiNET-developers.requirements.txt's stale URLisn't cosmetic:
pip install -r requirements.txtwas pullinggradienttracerfromwherever the old org name currently resolves to, not the actively developed repo.
CONTRACT.md: the GGUF tensor-naming/metadata contract (input_N,result,op.*params,general.description/name) any consumer needs — SKaiNET today,potentially a numcrux-hosted corpus later.
Test plan
gradienttracerURL)skainet-test-groundtruthmodule:TS-001goes from3/6 to 6/6 passing with these
op_paramsin place