[Doc] Fix init_options.md, and use nanobind docstrings for compileconfig - #873
Conversation
Move the three-layer offline_cache internals (KernelCompilationManager, PtxCache, PTX/LLVM-IR/SASS, ptxas/cuModuleLoadDataEx, driver compute cache, per-process nonce) into a clearly delimited "Under the hood (advanced)" subsection, and keep a concise user-facing offline_cache description. De-jargon the debug section (drop "IR verification / verifier walks the IR / compiler pass" wording). Resolves RULE 1 (undefined terms) and RULE 2 (internal material outside an advanced section) flagged by .github/workflows/check_doc_quality.yml.
…tions - Rule 2 (scope): drop the internal CompileConfig struct name and the C++ header "source of truth" link from the user-facing intro. - Rule 1 (term): define "primal" inline at its first occurrence in the ad_stack_experimental_enabled section. - Gloss "control-flow-graph optimization" for cfg_optimization so no bare compiler jargon remains outside the advanced section. - Restore the autodiff link to ./autodiff.md#what-can-go-wrong (matches the PR description; flagged by the Graphite AI review).
Add a docstring to every CompileConfig .def_rw in export_lang.cpp so each qd.init option is self-documenting at its nanobind binding site. The strings become the Python property __doc__ (visible in help() and IDE tooltips) and are emitted into the generated .pyi stub, with no extra build step or codegen. Keeps documentation next to where each option is exposed to Python, which is the common nanobind/pybind idiom.
Add an autoclass directive to init_options.md so the "All options" section is generated from CompileConfig's nanobind docstrings at build time, and set add_module_names = False so options display by short name. Drop the stale compile_config.h "source of truth" line from the intro.
The rendered page is viewed on a website built from a fixed revision, so it cannot guarantee it matches the reader's locally installed build.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c02c9afdc
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- opt_level: describe as the Quadrants IR optimization level (gates IR-level passes such as CSE), not an LLVM -O level. - debug: drop "adstack overflow" from the list of checks it enables; that check always runs regardless of debug. - debug_dump_path: it is the directory for the QD_DUMP_IR/QD_DUMP_CFG/QD_LOAD_IR env-driven dumps; the print_* options write to the current working directory. - init_options.md: the generated list is the compiler-config subset, not the full qd.init signature. Exclude default_up (derived from default_ip, rejected as a kwarg) and note the Python-side options (enable_fallback, src_ll_cache, log_level, require_version).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5320d488d0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- arch: defaults to qd.cpu when unspecified (init does not probe for the best accelerator). - gpu_max_reg / verbose_kernel_launches: document as currently having no effect (the config values are not wired into the JIT / launch path). - require_version: describe as a compatibility threshold (same major, at least the given minor.patch), not an exact match. - init_options.md: scope the QD_<NAME> env-var claim to "most" options; note the Python-side options are keyword-only (except log_level, via QD_LOG_LEVEL).
…aming - vk_api_version: document the expected "major.minor.patch" string format. - init_options.md: explain that each option is both a qd.init keyword argument and a read/write attribute on qd.cfg, so the list rendering as "property" of the config object reads naturally.
Sphinx autodoc can't read nanobind's type metadata (its nb_method getters carry no __annotations__), so the CompileConfig option list showed no type or default. Add a small conf.py bridge: patch PropertyDocumenter to emit :type: read from nanobind's own __nb_signature__, and append a Default line read from a default- constructed CompileConfig(), with a small override map for the few defaults that are machine-derived (arch, cpu_max_num_threads) or set on the Python side (offline_cache) or are dtypes (default_fp, default_ip).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d1030d5fd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
doc at htis point looks like https://genesis-embodied-ai.github.io/quadrants/config-nanobind-docstrings/user_guide/init_options.html#all-options 
|
Reflow the conf.py autodoc-bridge comments (previously wrapped ~80c) to 120c, and merge the three CompileConfig docstrings that fit on a single line under 120c (raise_on_templated_floats, check_out_of_bound, ad_stack_sparse_threshold_bytes) so they are no longer split needlessly.
- Initialize CompileConfig::print_ir_dbg_info{false} in the header. It was
uninitialized yet read on real compile paths (make_pass_printer), so the
generated docs published a nondeterministic default and the field was UB.
- Mark use_llvm and verbose as currently ineffective; neither field is read
anywhere (they only get a default assignment).
- Qualify print_kernel_asm by backend: native assembly on CPU, PTX on CUDA
(jit_cuda dumps compile_module_to_ptx output, not native SASS).
- Complete the Python-side qd.init option list in init_options.md with
print_non_pure, gdb_trigger, short_circuit_operators, print_full_traceback,
and unrolling_limit, and note which have QD_ environment variables.
The line-wrap CI (an LLM check) flags multi-line docstrings whose first line could hold more before 120. Repack every loose CompileConfig docstring so its non-final lines are packed near 120 (107-120c), and collapse ad_stack_size onto a single line. Pure re-wrapping; the concatenated docstring text is unchanged except a minor ad_stack_size reword.
…ewrites for init_options # Conflicts: # docs/source/user_guide/init_options.md
The doc-quality check flagged @qd.pure in init_options.md as an undefined term. @qd.pure is deprecated in favor of @qd.kernel(fastcache=True), so reword the print_non_pure option to use the current API, define what a "pure" kernel is, and link to the fastcache guide. Also update the same stale reference in the qd.init docstring.
…only Match the exact CompileConfig class name rather than any autoclass target under quadrants._lib.core, so future core bindings keep normal module qualification. Mirrors the class-name guards used elsewhere in conf.py.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ccd307802d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…rnel_profiler - demote_dense_struct_fors: CompileConfig::fit() force-sets it true on the Vulkan/Metal (SPIR-V) backends, so a user value is ignored there. - kernel_profiler: timing hooks are emitted only by CPU codegen and the CUDA/AMDGPU launch paths; SPIR-V emits none, so Vulkan/Metal collect no per-kernel timings.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01b6a7e042
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "GPU grid size to launch on the CUDA/AMDGPU backends (0 lets Quadrants pick based on occupancy). " | ||
| "Vulkan/Metal compute their dispatch grid automatically and ignore this.") |
There was a problem hiding this comment.
When a CUDA or AMDGPU reverse-mode task contains an adstack, this is not necessarily the grid size that gets launched: KernelLauncher::prepare_task() clamps effective_grid_dim to at most floor(65536 / block_dim) before dispatch. Consequently, increasing saturating_grid_dim beyond that cap has no effect for these kernels, so describe it as the normal/maximum grid target and mention the adstack-specific cap rather than promising the configured size is launched.
Useful? React with 👍 / 👎.
Reverse-mode CUDA/AMDGPU kernels carrying an autodiff stack are launched with a smaller grid: kernel_launcher.cpp clamps effective_grid_dim to floor(65536/block_dim) (kAdStackMaxConcurrentThreads) so the concurrent thread count stays bounded. Document it as a target/upper bound, not a guarantee.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac542727c5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The pass printer is invoked only at labeled checkpoints: compile_to_offloads() prints a handful (Initial IR, segment-reversed, adstack-size) and the printer created inside full_simplify() is never called, so passes like lower_ast, bit_loop_vectorize, offload, and cse_offloaded_tasks emit no snapshot. Reword print_ir accordingly and drop the now-inaccurate "per-pass" wording from print_ir_dbg_info.
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Doc-quality check flagged "native core" as undefined internal jargon. Reword to "Quadrants' compiled C++ runtime (a native crash rather than a Python exception)".
|
Ok, both doc quality passes and codex passes 🙌 Now I'll read through the doc myself. |
- offline_cache: reword the "fast warm-up" closing line. - num_compile_threads: explain it parallelizes a single kernel's internal tasks (not scheduling across kernels), answering the "back-to-back" confusion. - Split the option-list intro into two paragraphs so the qd.init/qd.cfg dual nature and the qd.python-backend note read separately; drop "frontend" jargon.
|
Ok, I reviewed the main init_options.md, and requested some changes. Next I'll wait for doc quality check to re-run, then if that passes, I'll merge. |
1 participant
Issue: #
Brief Summary
copilot:summary
Walkthrough
copilot:walkthrough