docs: time series demo notebook - #494
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new "Time Series" Jupyter notebook tutorial demonstrating Safe Synthesizer time-series synthesis (ECG200), including cu129 install, data conversion, generation, visualization, manual TSTR evaluation, and registers the notebook in the docs navigation. ChangesTime Series Tutorial
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: bc11103b-d513-4c78-99cf-0b53b93d967c
📒 Files selected for processing (2)
docs/tutorials/index.mddocs/tutorials/time-series.ipynb
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Typecheck
- GitHub Check: Analyze (Python)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{md,markdown,py}
📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)
**/*.{md,markdown,py}: Avoid decorative bold (**text**) in list items, body text, and docstrings; use structural cues (headers, list markers, colons, backticks) for emphasis instead
Use backticks for code identifiers, paths, and CLI commands in markdown and docstrings
Files:
docs/tutorials/index.md
**/*.{md,markdown}
📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)
**/*.{md,markdown}: Bold is acceptable only in markdown tables where it's the conventional way to mark header-like cells in the body
Use##headers to segment markdown sections instead of bold text
Use--(em-dash) instead of-(hyphen) for asides in markdown
Files:
docs/tutorials/index.md
docs/**/*.md
📄 CodeRabbit inference engine (.cursor/rules/writing-docs.mdc)
docs/**/*.md: Classify documentation content using the Diataxis framework (TUTORIAL, HOW-TO, EXPLANATION, or REFERENCE) before writing, ensuring each page fits exactly ONE type
Use cross-links between different Diataxis content types (TUTORIAL, HOW-TO, EXPLANATION, REFERENCE) to connect related documentation
Use MkDocs Material admonitions syntax for notes, warnings, and collapsible tips:!!! note,!!! warning,??? tip
Use MkDocs Material tabs syntax (=== "Label") for presenting multiple examples or implementations side-by-side
Use code blocks with metadata (title, hl_lines) to highlight relevant code snippets in documentation examples
Use Mermaid diagrams (flowchart, sequence diagrams, etc.) for visualizing architecture, workflows, and concepts in documentation
Write documentation following high signal-to-noise principles: every sentence must earn its place by providing essential information
Use progressive disclosure in documentation: start with simple concepts, then layer complexity for advanced readers
Include working code examples in documentation; ensure all code snippets are tested and actually work
List all prerequisites at the top of documentation pages before diving into main content
End documentation pages with 'Next steps' section containing links to related content and logical progression points
Files:
docs/tutorials/index.md
**/*
⚙️ CodeRabbit configuration file
**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.
- Refactor suggestion: use for local maintainability problems introduced
by the diff when they have clear future cost, such as duplicated setup,
unclear boundaries, over-mocking, avoidable complexity, or opaque test
helpers.- Nitpick: avoid in chill mode. Do not emit formatting, import-order,
wording, or style-only comments unless automated tools cannot catch the
issue and it affects maintainability.Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.
- Major: incorrect generation/training/evaluation behavior, broken
CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
cleanup and process-isolation bugs likely to fail CI or production
runs.- Minor: localized bugs, missing focused tests for changed behavior, or
bad test patterns that weaken regression coverage.- Trivial: small cleanup with no behavior impact. Usually suppress in
chill mode.- Info: context only. Avoid unless it helps reviewers understand risk.
Safe-Synthesizer-specific review focus: - Data ...
Files:
docs/tutorials/index.md
docs/**
⚙️ CodeRabbit configuration file
Review documentation as MkDocs Material content. Check Diataxis fit, accurate commands, internal links, code fences, and markdown style from STYLE_GUIDE.md.
Files:
docs/tutorials/index.md
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c4f7ab7f-9477-45d8-b8cc-8f7fa2e2bbbe
📒 Files selected for processing (1)
.github/workflows/config/.secrets.baseline
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Unit Tests (3.13)
- GitHub Check: Unit Tests (3.11)
- GitHub Check: Unit Tests (3.12)
- GitHub Check: Analyze (Python)
- GitHub Check: Analyze (python)
🧰 Additional context used
🪛 Betterleaks (1.2.0)
.github/workflows/config/.secrets.baseline
[high] 132-132: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 139-139: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (1)
.github/workflows/config/.secrets.baseline (1)
29-31: LGTM!Also applies to: 43-47, 54-56, 60-62, 78-80
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9e868ccf-dc7b-4b93-967e-938cb5915c21
📒 Files selected for processing (2)
docs/tutorials/time-series.ipynbmkdocs.yml
✅ Files skipped from review due to trivial changes (1)
- mkdocs.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Unit Tests (3.13)
- GitHub Check: Unit Tests (3.12)
- GitHub Check: Unit Tests (3.11)
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ CodeRabbit configuration file
**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.
- Refactor suggestion: use for local maintainability problems introduced
by the diff when they have clear future cost, such as duplicated setup,
unclear boundaries, over-mocking, avoidable complexity, or opaque test
helpers.- Nitpick: avoid in chill mode. Do not emit formatting, import-order,
wording, or style-only comments unless automated tools cannot catch the
issue and it affects maintainability.Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.
- Major: incorrect generation/training/evaluation behavior, broken
CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
cleanup and process-isolation bugs likely to fail CI or production
runs.- Minor: localized bugs, missing focused tests for changed behavior, or
bad test patterns that weaken regression coverage.- Trivial: small cleanup with no behavior impact. Usually suppress in
chill mode.- Info: context only. Avoid unless it helps reviewers understand risk.
Safe-Synthesizer-specific review focus: - Data ...
Files:
docs/tutorials/time-series.ipynb
docs/**
⚙️ CodeRabbit configuration file
Review documentation as MkDocs Material content. Check Diataxis fit, accurate commands, internal links, code fences, and markdown style from STYLE_GUIDE.md.
Files:
docs/tutorials/time-series.ipynb
🔇 Additional comments (7)
docs/tutorials/time-series.ipynb (7)
1-19: LGTM!
30-42: LGTM!
65-94: LGTM!
106-128: LGTM!
305-323: LGTM!
172-174: ⚡ Quick winNo issue. The parameter name
enforce_timeseries_fidelitymatches the actual API insrc/nemo_safe_synthesizer/config/generate.pyand is correct.> Likely an incorrect or invalid review comment.
1-353: ⚡ Quick winNavigation files have been properly updated. Both
mkdocs.yml(line 174) anddocs/tutorials/index.md(line 12) correctly reference the new time-series tutorial. No further action needed.
Greptile SummaryThis PR adds a new Jupyter notebook tutorial (
Confidence Score: 5/5Documentation-only change adding a tutorial notebook; no production code paths are touched. The change is entirely additive — a new Jupyter notebook, one line in the tutorial index, and one line in mkdocs.yml. No existing logic is modified, and the notebook follows the same structure and conventions as the other tutorials in the repo. No files require special attention. Important Files Changed
|
db66826 to
4699d8d
Compare
* Also address loading of params tweak * benchmark for methods using pretrained paths # Summary <!-- Brief description of changes --> ## Pre-Review Checklist <!-- These checks should be completed before a PR is reviewed, --> <!-- but you can submit a draft early to indicate that the issue is being worked on. --> Ensure that the following pass: - [x] `make format && make check` or via prek validation. - [x] `make test` passes locally - [x] `make test-e2e` passes locally - [ ] `make test-ci-container` passes locally (recommended) - [ ] GPU CI status check passes -- comment `/sync` on this PR to trigger a run (auto-triggers on ready-for-review) ## Pre-Merge Checklist <!-- These checks need to be completed before a PR is merged, --> <!-- but as PRs often change significantly during review, --> <!-- it's OK for them to be incomplete when review is first requested. --> - [x] New or updated tests for any fix or new behavior - [x] Updated documentation for new features and behaviors, including docstrings for API docs. ## Other Notes <!-- Please add the issue number that should be closed when this PR is merged. --> - Closes #540 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added `"auto"` schema method for structured generation, now the default. Intelligently selects `"structural_tag"` on compatible backends or falls back to `"regex"`. * Added `"structural_tag"` method option for structured generation with XGrammar support. * **Documentation** * Updated configuration reference and guides to reflect new `"auto"` default and additional schema method options. * **Chores** * Added XGrammar dependency (Linux-only) to optional dependencies. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: mkornfield <mkornfield@nvidia.com> Signed-off-by: Sean Yang <seayang@nvidia.com>
Signed-off-by: Sean Yang <seayang@nvidia.com> fixed grammar Signed-off-by: Sean Yang <seayang@nvidia.com> added notebook as false positive for secrets Signed-off-by: Sean Yang <seayang@nvidia.com> revert Signed-off-by: Sean Yang <seayang@nvidia.com> addressed feedback Signed-off-by: Sean Yang <seayang@nvidia.com> cleaned up Signed-off-by: Sean Yang <seayang@nvidia.com> fixed grammar Signed-off-by: Sean Yang <seayang@nvidia.com> added notebook as false positive for secrets Signed-off-by: Sean Yang <seayang@nvidia.com> revert Signed-off-by: Sean Yang <seayang@nvidia.com> cleaned up Signed-off-by: Sean Yang <seayang@nvidia.com>
4699d8d to
dcff1bf
Compare
kendrickb-nvidia
left a comment
There was a problem hiding this comment.
Let's discuss offline about how often a synthetic sequence collapses to all 0s.
Summary
Added time series demo notebook
Pre-Review Checklist
Ensure that the following pass:
make format && make checkor via prek validation.make testpasses locallymake test-e2epasses locallymake test-ci-containerpasses locally (recommended)/syncon this PR to trigger a run (auto-triggers on ready-for-review)Pre-Merge Checklist
Other Notes
Summary by CodeRabbit