docs: canonical contract-page pointers + rank-1 overclaim fixes - #119
Conversation
…ims (#118) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
Warning Review limit reached
Next review available in: 43 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR updates ONNX export documentation and docstrings with canonical HSSM contract links, published documentation URLs, and clarified concrete input-shape and rank requirements for SBI, BayesFlow, JAX, and LAN exporters. ChangesONNX contract alignment
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: 🔵 Low · up to This documentation-only change does not alter runtime behavior, but the current revision still needs a small formatting cleanup and explicit confirmation that the documented ONNX input-rank contract is consistent across exporters; it is mergeable with owner awareness and follow-up. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
Updates LANfactory’s ONNX-related documentation and docstrings to point at HSSM’s canonical ONNX likelihood contract page and to correct earlier overclaims about “rank-1” being the ecosystem-wide invariant (clarifying that the real invariant is concrete/static input dims; rank depends on the exporter/tracer).
Changes:
- Repoint ONNX contract references in exporter docstrings and docs to HSSM’s canonical contract page.
- Reword sbi/bayesflow export docs to clarify why some exporters must be rank-1 (input slicing) while LAN exporters are typically rank-2
(1, D)(Gemm lowering). - Update HSSM consumption links in tutorials to use rendered docs URLs instead of GitHub blobs.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lanfactory/onnx/sbi.py | Docstring clarifies rank vs “concrete dims” invariant and corrects prior “LAN-and-HSSM convention” wording. |
| src/lanfactory/onnx/jax_export.py | Docstring now cites the canonical HSSM ONNX contract page instead of an internal/unreachable reference. |
| docs/tutorials/exporting_sbi_to_onnx.ipynb | Tutorial text corrected to explain rank-1 necessity for slicing graphs; adds canonical contract link. |
| docs/tutorials/exporting_bayesflow_to_onnx.ipynb | Same clarification as sbi tutorial; updates consumption links to rendered HSSM docs. |
| docs/index.md | Adds canonical ONNX contract link near ONNX export guidance. |
| docs/exporting_sbi_models.md | Adds canonical ONNX contract link as the artifact rules reference. |
| docs/exporting_bayesflow_models.md | Adds canonical ONNX contract link and updates an HSSM tutorial link to rendered docs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| The exported graph is a single concatenated single-trial input of | ||
| **rank 1, shape ``(theta_dim + x_dim,)``** (the contract invariant is concrete | ||
| dims — rank is tracer-specific; the LAN exporters emit rank-2 ``(1, D)`` Gemm). Inside the graph the | ||
| input is split into ``theta`` and ``x``, upranked to ``(1, …)`` to satisfy | ||
| sbi's batched ``log_prob`` API, and routed through the trained estimator. |
There was a problem hiding this comment.
Done — paragraph reflowed; 'Inside the graph…' now starts its own sentence.
| The export follows the ecosystem's single-trial ONNX contract (canonical | ||
| statement: https://lnccbrown.github.io/HSSM/how_to/custom_onnx_likelihoods/): the graph is traced with a **concrete** ``(1, input_dim)`` dummy | ||
| and no dynamic axes, exactly like the torch MLP exporter and the production |
There was a problem hiding this comment.
Done — the URL now sits on its own line.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lanfactory/onnx/jax_export.py`:
- Around line 8-9: Wrap the changed docstring text to the required line length
in both sites: in src/lanfactory/onnx/jax_export.py lines 8-9, split the URL and
single-trial ONNX contract description across lines; in
src/lanfactory/onnx/sbi.py lines 12-14, split the rank description across lines
without changing its content.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f050d1f1-d47e-443f-8117-d03dc5be19d5
📒 Files selected for processing (7)
docs/exporting_bayesflow_models.mddocs/exporting_sbi_models.mddocs/index.mddocs/tutorials/exporting_bayesflow_to_onnx.ipynbdocs/tutorials/exporting_sbi_to_onnx.ipynbsrc/lanfactory/onnx/jax_export.pysrc/lanfactory/onnx/sbi.py
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/lanfactory/onnx/sbi.py:16
- In this docstring paragraph, “Inside the graph …” is currently appended to the previous sentence on a very long line, which hurts readability and can render awkwardly in generated docs. Reflow this section so “Inside the graph …” starts a new sentence/line and the paragraph wraps consistently.
The exported graph takes a single concatenated single-trial input of
**rank 1, shape ``(theta_dim + x_dim,)``**. The contract invariant is concrete
dims; rank is tracer-specific (the LAN exporters emit rank-2 ``(1, D)``
``Gemm`` graphs). Inside the graph the input is split into ``theta`` and ``x``, upranked to ``(1, …)`` to satisfy
sbi's batched ``log_prob`` API, and routed through the trained estimator.
Companion to lnccbrown/HSSM#1178 (docs cluster 9).
jax_export.pydocstring cited "HSSMSpine CLAUDE.md" — unreachable for library users; now cites the canonical page (https://lnccbrown.github.io/HSSM/how_to/custom_onnx_likelihoods/).sbi.pydocstring attributed its rank-1 input to "the LAN-and-HSSM convention" — the LAN exporters emit rank-2 (1, D) Gemm; reworded to state the actual invariant (concrete dims; rank is tracer-specific).exporting_sbi_to_onnx.ipynb: "the rank-1 contract is what lets HSSM vmap" corrected — rank-1 is forced by the flow/ratio graph slicing its input; the invariant is concrete dims. Canonical link added.exporting_bayesflow_to_onnx.ipynb: "same contract as the sbi and LAN exporters" fixed the same way; consumption stub now links HSSM's rendered docs instead of a GitHub blob.exporting_*_models.mdguides +docs/index.mdgain the canonical contract-page link (previously zero occurrences repo-wide).mkdocs build --strictgreen.Closes #118
🤖 Generated with Claude Code
Summary by CodeRabbit