[Docs] Add init_options.md user-guide page documenting commonly-tuned qd.init knobs#581
[Docs] Add init_options.md user-guide page documenting commonly-tuned qd.init knobs#581duburcqa wants to merge 2 commits into
Conversation
|
@claude review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29d34a9a5f
ℹ️ 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".
There was a problem hiding this comment.
LGTM — focused CUDA perf knob with safe default and good unit test coverage.
Extended reasoning...
Overview
This PR adds a single field CompileConfig::cuda_jit_opt_level (default -1) that, when set in [0, 4], is forwarded to cuModuleLoadDataEx via CU_JIT_OPTIMIZATION_LEVEL. The option-array assembly in JITSessionCUDA::add_module is refactored into a free function populate_module_load_jit_options plus a backing struct CudaModuleLoadJitOptions, which makes the logic unit-testable without a CUDA driver. Five new gtests cover the matrix of (max_reg present? × opt_level present?) plus a storage-stability check and the upper boundary.
Security risks
None identified. The new knob is a compile-time perf tradeoff for the in-driver PTX→SASS JIT — no auth, crypto, IPC, or untrusted-input surface. The accepted range is asserted ([0, 4]); the option is omitted entirely when the value is -1, preserving prior behavior. The struct's int_storage correctly extends the lifetime of the scalars whose addresses are handed to the driver, eliminating the stack-pointer footgun the previous inline code relied on.
Level of scrutiny
Low-to-medium. The change is small, additive, and gated by a default that produces the exact pre-PR option array (num_options=0 or num_options=1 with only CU_JIT_MAX_REGISTERS). The refactor is mechanical and the new tests pin down the contract. The blast radius is limited to CUDA cold compile.
Other factors
The two bug reports filed are non-blocking: (1) the docs/ omission is a nit because the sibling knobs gpu_max_reg, cuda_stack_limit, and num_compile_threads are also undocumented, so this matches the established repo pattern; (2) the missing #pragma once on jit_cuda.h is pre-existing and currently latent (only two TUs include it, no transitive cycle). Neither warrants delaying this PR.
|
Sounds good. Thoughts on how we add this to user-facing documentation? (Not averse to having a link to a code file showing all available options, otheriwe maybe a standard .md file?) |
…icitly so configuration typos like cuda_jit_opt_level=-2 fail loudly at init instead of silently falling back to the driver default - addresses codex bot review feedback on PR #581
…icitly so configuration typos like cuda_jit_opt_level=-2 fail loudly at init instead of silently falling back to the driver default - addresses codex bot review feedback on PR #581
…uda_jit_opt_level (and QD_CUDA_JIT_OPT_LEVEL) - what it does, accepted values, when to use it, and how it relates to offline_cache - hooked into the user guide's Performance section, addresses claude / codex bot review feedback on PR #581 about missing user-facing docs for the new public-API knob
…uda_jit_opt_level (this PR's new knob), plus widely-used compile-time and debug knobs that Genesis __init__ exercises (cfg_optimization, fast_math, num_compile_threads, cpu_max_num_threads, debug, check_out_of_bound) - drops the gpu_max_reg / cuda_stack_limit sections since they are rarely used and documenting every CompileConfig field is a rabbit hole, addressing user follow-up review feedback on PR #581
…uda_jit_opt_level (this PR's new knob), plus widely-used compile-time and debug knobs that Genesis __init__ exercises (cfg_optimization, fast_math, num_compile_threads, cpu_max_num_threads, debug, check_out_of_bound) - drops the gpu_max_reg / cuda_stack_limit sections since they are rarely used and documenting every CompileConfig field is a rabbit hole, addressing user follow-up review feedback on PR #581
…ZATION_LEVEL through CompileConfig and qd.init / QD_CUDA_JIT_OPT_LEVEL, with a populate_module_load_jit_options helper that encodes scalar-int values via (void*)(uintptr_t)value (the cuModuleLoadDataEx convention) and unit tests pinning that contract; documentation page init_options.md covers cuda_jit_opt_level, offline_cache and the commonly-tuned compile vs runtime / debug trade-off knobs
…graph (no Genesis mention, no recently-added framing), drop the per-section subheadings for the existing compile-time knobs in favor of a tighter bullet list, and simplify the debug / check_out_of_bound sections; addresses user follow-up review feedback on PR #581
…ZATION_LEVEL through CompileConfig and qd.init / QD_CUDA_JIT_OPT_LEVEL, with a populate_module_load_jit_options helper that encodes scalar-int values via (void*)(uintptr_t)value (the cuModuleLoadDataEx convention) and unit tests pinning that contract; init_options.md user-guide page extended with the cuda_jit_opt_level section, the commonly-tuned compile vs runtime trade-off knobs, and a clearer offline_cache description framed around cold-start emulation across Python invocations
…ZATION_LEVEL through CompileConfig and qd.init / QD_CUDA_JIT_OPT_LEVEL with a populate_module_load_jit_options helper that encodes scalar-int values via (void*)(uintptr_t)value (the cuModuleLoadDataEx convention) and unit tests pinning that contract; init_options.md user-guide page extended with the cuda_jit_opt_level section, the commonly-tuned compile vs runtime / debug trade-off knobs
…ZATION_LEVEL through CompileConfig and qd.init / QD_CUDA_JIT_OPT_LEVEL with a populate_module_load_jit_options helper that encodes scalar-int values via (void*)(uintptr_t)value (the cuModuleLoadDataEx convention) and unit tests pinning that contract; init_options.md user-guide page extended with the cuda_jit_opt_level section, the commonly-tuned compile vs runtime / debug trade-off knobs
…LINE_CACHE=0 instead of QD_CUDA_JIT_OPT_LEVEL=2) so the two PRs ship the same opening paragraph and the integration merge is conflict-free; addresses user follow-up review feedback on PR #581
…at the default for production' line from init_options.md cuda_jit_opt_level section; the driver default is conservative and lower levels are often the better point on the curve once the SASS-quality vs compile-time trade-off has been measured for the workload, addresses user follow-up review feedback on PR #581
…nto a single sentence describing the runtime-vs-compile-time trade-off and the prohibitive-compile-time scenario it targets (reverse-mode autodiff on very large kernels), addresses user follow-up review feedback on PR #581
…paragraphs into a single sentence describing the runtime-vs-compile-time trade-off and the reverse-mode-autodiff-on-large-kernels scenario where ptxas dominates total CUDA compile time, addresses user follow-up review feedback on PR #581
…ZATION_LEVEL through CompileConfig and qd.init / QD_CUDA_JIT_OPT_LEVEL with a populate_module_load_jit_options helper that encodes scalar-int values via (void*)(uintptr_t)value; init_options.md user-guide page extended with the cuda_jit_opt_level section
…ntro paragraph: split the runtime-vs-compile-time trade-off into one sentence and the ptxas-share-of-CUDA-compile-time observation into a separate one starting with 'Typically', addresses user follow-up review feedback on PR #581
…ZATION_LEVEL through CompileConfig and qd.init / QD_CUDA_JIT_OPT_LEVEL with a populate_module_load_jit_options helper that encodes scalar-int values via (void*)(uintptr_t)value; init_options.md user-guide page extended with the cuda_jit_opt_level section
…o the ones above' meta-commentary line from init_options.md; user-facing docs should not narrate the page's history, addresses user follow-up review feedback on PR #581
…ack into one ### subsection per arg in init_options.md so each option has its own anchor and a parallel structure with the cuda_jit_opt_level / debug / check_out_of_bound sections, addresses user follow-up review feedback on PR #581
…ZATION_LEVEL through CompileConfig and qd.init / QD_CUDA_JIT_OPT_LEVEL with a populate_module_load_jit_options helper that encodes scalar-int values via (void*)(uintptr_t)value; init_options.md user-guide page extended with the cuda_jit_opt_level section
…_out_of_bound section headings and into the body so all sections in init_options.md follow the same shape (heading is just the option name; default value stated in the body), addresses user follow-up review feedback on PR #581
…ZATION_LEVEL through CompileConfig and qd.init / QD_CUDA_JIT_OPT_LEVEL with a populate_module_load_jit_options helper that encodes scalar-int values via (void*)(uintptr_t)value; init_options.md user-guide page extended with the cuda_jit_opt_level section
Coverage Report (
|
| File | Coverage | Missing |
|---|
Diff coverage: 0% · Overall: 61% · 0 lines, 0 missing
…ZATION_LEVEL through CompileConfig and qd.init / QD_CUDA_JIT_OPT_LEVEL with a populate_module_load_jit_options helper that encodes scalar-int values via (void*)(uintptr_t)value; init_options.md user-guide page extended with the cuda_jit_opt_level section
…ZATION_LEVEL through CompileConfig and qd.init / QD_CUDA_JIT_OPT_LEVEL with a populate_module_load_jit_options helper that encodes scalar-int values via (void*)(uintptr_t)value; init_options.md user-guide page extended with the cuda_jit_opt_level section
…ZATION_LEVEL through CompileConfig and qd.init / QD_CUDA_JIT_OPT_LEVEL with a populate_module_load_jit_options helper that encodes scalar-int values via (void*)(uintptr_t)value; init_options.md user-guide page extended with the cuda_jit_opt_level section
…ZATION_LEVEL through CompileConfig and qd.init / QD_CUDA_JIT_OPT_LEVEL with a populate_module_load_jit_options helper that encodes scalar-int values via (void*)(uintptr_t)value; init_options.md user-guide page extended with the cuda_jit_opt_level section
117f6bb to
15ce898
Compare
…backend gating per review
Coverage Report (
|
| File | Coverage | Missing |
|---|
Diff coverage: 0% · Overall: 73% · 0 lines, 0 missing
TL;DR
docs/source/user_guide/init_options.md. Documentscfg_optimization,fast_math,num_compile_threads,debug,check_out_of_bound. Each section explains what the knob does and the context in which a user should consider setting it (compile-time vs runtime trade-off, profiling, debugging).Why
CompileConfighas dozens of fields and the page documents only the ones that come up regularly. The cuda-specificcuda_jit_opt_levelknob from the original revision of this PR was dropped because the cold-compile A/B against the integration branch found no measurable benefit from setting it, so exposing it would add a public API knob with no payoff.Tests
Doc-only change. No tests touched.