Repository navigation
Docs: add Concepts section (config system, representations, model assembly, trainer internals) - #90
Merged
Merged
Conversation
…embly, trainer internals) Explanation layer per the docs design: four pages + index, linked from the root toctree (which is also reordered to the target navigation: Installation -> Tutorial -> User Guide -> Concepts -> Examples -> API). - config_system: the three config layers, CFG dual view + recursive update, the copy()-vs-deepcopy() pitfall (copy() returns a plain dict), DEFAULTS (dtype, single-stream RNG, lazy device), and the 0.0.32 checkpoint regression case study incl. the make_safe_globals silent-drop -> RuntimeWarning lesson - representations: lead-first signals + Standard12Leads, waveform masks + class_map, mask_to_intervals / masks_to_waveforms with the ECGWaveForm dataclass, (generalized) interval algebra, multi-hot labels - architecture: building-block configs -> BACKBONES -> task models layering, the registry table, the compute_output_shape contract, the tensor-vs-Output-dataclass output conventions, and the system workflow tikz figure moved here from the landing page - training_internals: loop anatomy (run_one_step -> criterion by registry, monitor / early stopping / keep_checkpoint_max / resume), the seeding machinery (seed_everything + worker_init_fn, and why forked workers would otherwise share one RNG stream), and where augmentation lives Every code block was executed before being written down. Also pin the pre-commit hook interpreter to python3.11 (the autoupdated isort requires >= 3.11 while the system default is 3.10), restoring local hook runs.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #90 +/- ##
==========================================
- Coverage 93.47% 92.76% -0.71%
==========================================
Files 138 138
Lines 18628 18628
==========================================
- Hits 17412 17280 -132
- Misses 1216 1348 +132
Flags with carried forward coverage won't be shown. Click here to find out more. |
The rule 'svg { width: 80% }' (added 2023, when inline SVGs in pages
were content figures) now hits the fontawesome SVG replacements that
all.min.js produces for every theme icon: sidebar chevrons, header
sidebar-toggle buttons, etc. get stretched to 80% of their container,
which visually empties the left sidebar and wraps the 'On this page'
toggle label onto a second line.
Scope the rule to figures inside the article body, where it was always
meant to apply (tikz workflow diagrams); theme-chrome SVG icons keep
their intrinsic 1em sizing.
Found by viewing the local build served through /var/www.
Review of the hand-me-down custom.css against the pydata theme's own styles: - figure rule: use max-width instead of width so small figures keep their intrinsic size (width: 80% upscales them), plus height: auto - .skip-link override removed: the theme already implements the skip link (fixed, slid out of view, slides in on focus); our absolute positioning fought it with a weaker position model and a garbled rationale comment - kept as deliberate style choices: the pink caption color and the left-aligned display math with right-floated equation numbers (both live: package docstrings contain 38 math directives/roles)
The pydata theme reserves 25% of the page width for the primary sidebar. On top-level leaf pages (install, tutorial, examples, changelog, history) and the special index pages (genindex, search, py-modindex) the section navigation is empty -- the sidebar shows only the search box and a bare 'Section Navigation' heading, yet still consumes a quarter of the layout. Hide it with :has(.bd-sidenav) targeting, mirroring the theme's own hide-on-wide class at its 960px breakpoint; the flex layout lets the article span the reclaimed width. Any future page with an empty section navigation is handled automatically; the off-canvas drawer on narrow screens is unaffected.
Sweep the concept pages for casual/journalistic phrasing, following the same standard applied to the user guide (declarative sentences, no anthropomorphism, no chatty verbs): - 'what happens inside the trainer' -> 'the internals of the trainer' - 'Everything ... is driven by' -> 'All components ... are configured through'; 'copy() vs deepcopy() -- a real pitfall' -> 'copy() versus deepcopy()'; 'is what makes runs reproducible' -> 'provides run-to-run reproducibility'; 'costs time' -> 'adds overhead'; 'is a general lesson' -> 'generalizes beyond this case'; em-dash aside 'The pattern -- turn ... --' -> plain relative clause - 'Sticking to them lets ... compose freely' -> 'These conventions allow ... to interoperate'; 'cut training segments' -> 'extract'; 'never inside the model' -> 'rather than inside the model' - 'a decorator away' -> 'requires only a decorator'; 'asks its backbone' -> 'obtains the feature shape from its backbone'; 'you can sanity check a config in one line' -> 'a configuration can be checked without executing it'; 'ECG_CRNN and friends' -> 'such as ECG_CRNN'; 'without touching' -> 'without modifying'; 'backbone + attention + head' -> 'a backbone, attention and head'; heading 'tensors today, dataclasses going forward' -> 'tensors and output dataclasses' - 'saves hours of debugging' -> dropped; 'the base class owns the loop' -> 'implements the loop'; 'never code' -> 'without code changes'; 'walks this decision path' -> 'applies the following decision logic'; 'never sees unaugmented data' -> 'receives only augmented data'; 'for, say,' -> 'for transforms such as'; 'is what keeps ... composable without API churn' -> 'preserves composability ... without changes to the calling convention'
This was referenced Oct 29, 2026
The python3.11 pin broke the check-formatting runner: the job sets up Python 3.12 only and the ubuntu-22.04 image has no bare python3.11 binary, so pre-commit could not create the isort hook environment (virtualenv -p python3.11). The original reason for the pin was the local default being 3.10; that premise is gone (local python3 is now 3.13, and the package floor is >= 3.11), so 'python3' resolves to a compatible interpreter on both sides: the runner's setup-python 3.12 and local 3.13.
kjs11
enabled auto-merge
November 2, 2026 09:48
kjs11
approved these changes
Nov 2, 2026
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
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.
What
The explanation layer of the docs (step 1 of the docs plan): a new "Concepts" section with four pages + index, linked from the root toctree.
self.config), theCFGdual dict+attribute view and its recursiveupdate, the realcopy()-vs-deepcopy()pitfall (dict.copyreturns a plain dict and loses the attribute view),DEFAULTS(dtype, single-stream RNG, lazy device probing and why), and the 0.0.32 checkpoint regression as a case study — closing with themake_safe_globalssilent-drop →RuntimeWarninglesson.Standard12Leads, waveform masks +class_map,mask_to_intervals(half-open semantics) andmasks_to_waveforms(ECGWaveFormdataclass), the interval / generalized-interval algebra, multi-hot labels and inference-time binarization.BACKBONES→ task models layering, the registry map, the compute_output_shapecontract, and the two output conventions (classic models return tensors; benchmark models adopt thecomponents.outputs` dataclasses). The system workflow tikz figure moves here from the landing page.run_one_step→ criterion assembled fromtrain_config.lossvia theLOSSESregistry, monitor / early stopping /keep_checkpoint_max/ resume), the determinism machinery (seed_everything+worker_init_fn, and why forked DataLoader workers would otherwise share one RNG stream), and where augmentation lives (workers, two-argument(sig, label)contract).Verification
ECGWaveFormfields,one_hot_encodeshapes,ensure_lead_fmt,compute_output_shape→(None, 4), seeding reproducibility); two claims discovered by that verification and written accordingly:copy()returns a plain dict, andECG_CRNNreturns raw tensors while the Output-dataclass contract is used by the newer benchmark models.Also included
.pre-commit-config.yaml: pindefault_language_versiontopython3.11— the autoupdated isort hook requires ≥ 3.11 while the system default is 3.10, which had forced--no-verifycommits. The pin restores local hook runs (this very commit went through the hooks locally).