Repository navigation
Release 0.9.0 - #156
Release 0.9.0#156
Conversation
JaxMLP.setup() dropped "linear" entries from activation_funs while __call__ indexed that list by layer position, so any "linear" before the output layer raised IndexError, or silently applied the wrong activation when activations was longer than layer_sizes. Map "linear" to an identity and keep one activation per layer. The identity adds nothing to the traced graph, so the exported ONNX for existing configs is byte-identical. Closes #105 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LRVhzVTfUsMkPu83XJbh8E
The alignment test only used one activation per layer, so nothing guarded the over-long lists jaxtrain produces when a YAML already lists the output activation (e.g. ["tanh", "tanh", "linear", "linear"] for three layers): rewriting the loop with zip(strict=True) passed the whole suite. Add that case and make the reference loop walk the layers, ignoring entries past the last one. An activations list shorter than layer_sizes still failed with issue #105's "IndexError: tuple index out of range" from inside flax. JaxMLP.setup() now raises a ValueError naming both lengths. Over-long lists are unaffected. Document both rules in the JaxMLP docstring. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01HV5AjSmHRfcHmi7nM3ERV1
…n-index-misalignment Resolve the tests/test_jax_mlp.py import conflict with the ruff 0.17 migration (#155): keep main's sorted imports and add numpy, which the layer-alignment test uses.
No test covered the one config class whose output changes in this PR: lists longer than layer_sizes with a hidden "linear", which main ran without an error but with shifted activations (issue #105's silent case). Mutation testing found two plausible regressions surviving the suite because of it: the output layer reading activations[-1], and extra entries applied after the output layer. Add two such cases, which fail on main with wrong outputs and catch both regressions, and give the parametrized cases readable IDs. The JaxMLP docstring said entries past the last layer are ignored, but they are still looked up, so a misspelled extra raises KeyError. Say they are not applied but must still be valid names. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01LRVhzVTfUsMkPu83XJbh8E
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…linear" Included in 0.9.0 at the maintainer's request; closes #105. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Linked Issues check |  |
The change satisfies #105. src/lanfactory/trainers/jax_mlp.py maps "linear" to an identity function and keeps one activation function per supplied activation. The forward pass applies `activation_… |
| Out of Scope Changes check | |
The reviewed changes stay within the stated release and #105 scope. The version updates implement the declared 0.9.0 release. The JAX implementation, documentation, and tests support the activation-al… |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files. (1 skipped: 1 … |
| Description Check | |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly identifies the primary release and matches the version changes in the pull request. |
Full details: Linked Issues check
-
Explanation
The change satisfies
#105.src/lanfactory/trainers/jax_mlp.pymaps"linear"to an identity function and keeps one activation function per supplied activation. The forward pass appliesactivation_funs[i]tolayers[i], so hidden"linear"entries remain layer-aligned. The setup check rejects too-short activation lists with both counts.tests/test_jax_mlp.pyadds coverage for standard, hidden-linear, overlong, and too-short activation lists.
Full details: Out of Scope Changes check
-
Explanation
The reviewed changes stay within the stated release and
#105scope. The version updates implement the declared 0.9.0 release. The JAX implementation, documentation, and tests support the activation-alignment requirement. No unrelated source change is identified.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
There was a problem hiding this comment.
🟢 Approval recommended
The focused fix has targeted regression coverage, version updates are consistent, and static review identified no blocking issues.
No open issues 🎉
What changed in this PR
Prepares LANfactory 0.9.0 and fixes JAX MLP activation alignment, addressing #105.
Changes:
- Updates package and runtime versions to 0.9.0.
- Preserves per-layer activations, including hidden
"linear"entries, and rejects insufficient activation lists. - Adds regression tests for alignment and activation-count validation.
| File | Description |
|---|---|
tests/test_jax_mlp.py |
Tests activation alignment and insufficient activation lists. |
src/lanfactory/trainers/jax_mlp.py |
Fixes activation mapping and documents validation behavior. |
src/lanfactory/__init__.py |
Sets runtime version to 0.9.0. |
pyproject.toml |
Sets package version to 0.9.0. |
Review effort: Balanced
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
dummy_generator_config_simple_two_choices never reset its two_choices flag between draws, so a non-two-choice model could follow an earlier rejected two-choice one and get through. That is how run_tests (3.13) on #156 drew lba_3_vs_constraint and failed with 'Drift rates do not sum to 1'. Both flags are now re-evaluated for every candidate. The two *_vs_constraint LBA models are also left out of the random pool: the default sampler draws drift rates independently, so they can never generate training data. A new test draws from the fixture 200 times; it fails on the old fixture and passes now. Co-Authored-By: Claude Opus 5.5 <[email protected]>
pyproject.tomlandsrc/lanfactory/__init__.py."linear"). Merging this PR also merges Keep JAX MLP activations aligned with layers for hidden "linear" #153.dummy_generator_config_simple_two_choicesnever reset its two-choice flag between draws, so it could hand a three-choice constrained LBA model to the 3.13 job. The two unsampleable*_vs_constraintLBA models also leave the random pool, and a new test guards the fixture.0.9.0onmainand draft the GitHub release; publishing it triggers the PyPI upload.Closes #105
🤖 Generated with Claude Code